timedelta for total running time

[PhilipDeegan]

No description provided.

[coderabbitai]

📝 Walkthrough

Walkthrough

Modified the time-tracking and output formatting logic in Simulator.run to introduce a cumulative timer (tot), compute per-iteration elapsed deltas from that cumulative value, and log structured "Starting at" and "Finished at" timestamps alongside per-iteration elapsed time instead of raw summed performance metrics.

Changes

Cohort / File(s)	Summary
Time-tracking and Output Formatting pyphare/pyphare/simulator/simulator.py	Added cumulative timing variable tot to accumulate per-iteration durations. Replaced uses of np.sum(perf) with a delta derived from tot for per-iteration prints and final total. Added "Starting at" and "Finished at" timestamp logs and refactored iteration output formatting.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	❓ Inconclusive	No pull request description was provided by the author, making it impossible to assess if the description relates to the changeset.	Add a pull request description explaining the changes, such as why timedelta was introduced and what benefits it provides to the simulator logging.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'timedelta for total running time' accurately describes the main change: modifying how total running time is calculated and displayed using timedelta values.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pyphare/pyphare/simulator/simulator.py (1)

219-224: Compute delta only on rank 0 to avoid per-rank overhead.

delta is only used in the rank-0 print path, so Line 219 can be moved inside that branch.

♻️ Proposed micro-optimization

-            delta = datetime.timedelta(seconds=tot)
             if cpp.mpi_rank() == 0:
+                delta = datetime.timedelta(seconds=tot)
                 print(
                     f"t = {t:8.5f} - {ticktock:6.5f}sec - total {delta}",
                     end=self.print_eol,
                 )

Based on learnings: PhilipDeegan has indicated a preference for minimal and efficient code, even after making changes to get a function working.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyphare/pyphare/simulator/simulator.py` around lines 219 - 224, Move the
computation of delta (currently created via datetime.timedelta(seconds=tot))
into the rank-0 branch so it is only constructed when needed: inside the if
cpp.mpi_rank() == 0: block right before the print call that uses delta. Update
references to delta accordingly and keep tot as-is; this eliminates per-rank
overhead by computing datetime.timedelta(seconds=tot) only on the process that
prints.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pyphare/pyphare/simulator/simulator.py`:
- Around line 219-224: Move the computation of delta (currently created via
datetime.timedelta(seconds=tot)) into the rank-0 branch so it is only
constructed when needed: inside the if cpp.mpi_rank() == 0: block right before
the print call that uses delta. Update references to delta accordingly and keep
tot as-is; this eliminates per-rank overhead by computing
datetime.timedelta(seconds=tot) only on the process that prints.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d7d962d and 43ba2a6.

📒 Files selected for processing (1)

• pyphare/pyphare/simulator/simulator.py

[coderabbitai]

🧹 Nitpick comments (1)

pyphare/pyphare/simulator/simulator.py (1)

227-228: Consider timezone-aware ISO timestamps for logs.

Using UTC ISO-8601 timestamps makes logs easier to correlate across machines and parsers.

💡 Optional refactor

-        print_rank0("Starting at ", datetime.datetime.now())
+        started_at = datetime.datetime.now(datetime.timezone.utc)
+        print_rank0("Starting at", started_at.isoformat())

...

-        print_rank0("Finished at ", datetime.datetime.now())
+        finished_at = datetime.datetime.now(datetime.timezone.utc)
+        print_rank0("Finished at", finished_at.isoformat())

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pyphare/pyphare/simulator/simulator.py` around lines 227 - 228, Replace the
naive timestamp print with a timezone-aware ISO-8601 UTC timestamp: update the
print_rank0 call that currently uses datetime.datetime.now() to call
datetime.datetime.now(datetime.timezone.utc).isoformat() (or equivalently use
datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc).isoformat()),
so the line referencing print_rank0 and datetime.datetime.now becomes a UTC ISO
timestamp; keep the total advance time print using datetime.timedelta as-is or
convert/label it if you want it explicitly tied to UTC.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pyphare/pyphare/simulator/simulator.py`:
- Around line 227-228: Replace the naive timestamp print with a timezone-aware
ISO-8601 UTC timestamp: update the print_rank0 call that currently uses
datetime.datetime.now() to call
datetime.datetime.now(datetime.timezone.utc).isoformat() (or equivalently use
datetime.datetime.utcnow().replace(tzinfo=datetime.timezone.utc).isoformat()),
so the line referencing print_rank0 and datetime.datetime.now becomes a UTC ISO
timestamp; keep the total advance time print using datetime.timedelta as-is or
convert/label it if you want it explicitly tied to UTC.

---

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 43ba2a6 and 869eebd.

📒 Files selected for processing (1)

• pyphare/pyphare/simulator/simulator.py

[PhilipDeegan]

np.sum(perf) seems like a bad idea in terms of imbalance during advances