phlop runtime process monitoring #881
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe recent changes in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
Files skipped from review due to trivial changes (1)
- src/simulator/simulator.hpp
Additional comments not posted (3)
tools/python3/plotting.py (1)
27-27: Verify the impact of removingsingle_hier_for_all_quantities.The
Runobject is now instantiated without thesingle_hier_for_all_quantities=Trueparameter. Ensure that this change does not adversely affect theRunclass's behavior or its interactions with other components.tools/python3/phloping.py (1)
126-143: Improved normalization logic.The changes to the
normalised_times_for_Lmethod enhance the accuracy of time normalization by correctly handling particle counts. This is a valuable improvement.pyphare/pyphare/simulator/simulator.py (1)
Line range hint
174-205:
Verify integration of monitoring feature.The
runmethod now includes monitoring setup and shutdown. Ensure that the monitoring feature integrates seamlessly and does not interfere with the core simulation logic.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- pyphare/pyphare/simulator/simulator.py
- src/simulator/simulator.hpp
- tools/python3/plotting.py
Additional context used
Ruff
pyphare/pyphare/simulator/monitoring.py
1-1:
osimported but unusedRemove unused import:
os(F401)
8-8:
phlop.dict.ValDictimported but unused; consider usingimportlib.util.find_specto test for availability(F401)
49-49:
phlop.app.stats_manimported but unusedRemove unused import:
phlop.app.stats_man(F401)
GitHub Check: CodeQL
pyphare/pyphare/simulator/monitoring.py
[notice] 1-1: Unused import
Import of 'os' is not used.
[notice] 12-12: Statement has no effect
This statement has no effect.
Additional comments not posted (3)
pyphare/pyphare/simulator/monitoring.py (2)
28-31: LGTM!The function correctly constructs the YAML file path and ensures the directory exists.
16-22: Remove unused import invaldict.The import statement for
ValDictis flagged as unused. Consider importing only whenhave_phlop()returnsTrue.- from phlop.dict import ValDict + if have_phlop(): + from phlop.dict import ValDict + return ValDict(**kwargs)Likely invalid or redundant comment.
tools/python3/phloping.py (1)
126-143: LGTM!The changes to trim initial particle counts improve the accuracy of normalization.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- pyphare/pyphare/simulator/monitoring.py
- pyphare/pyphare/simulator/simulator.py
- src/simulator/simulator.hpp
- tools/python3/plotting.py
Additional comments not posted (1)
tools/python3/phloping.py (1)
126-143: LGTM! Verify array length handling.The changes improve the normalization accuracy by trimming the initial particle count when necessary.
Ensure that the lengths of
timesandLi_timesarrays are correctly handled throughout the codebase.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
pyphare/pyphare/simulator/monitoring.py (1)
13-19: Add a docstring to clarify the function's purpose.Consider adding a docstring to
valdictto explain its functionality and usage.def valdict(**kwargs): """ Returns a ValDict if available, otherwise returns a standard dictionary. """ if not have_phlop(): return dict from phlop.dict import ValDict return ValDict(**kwargs)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- pyphare/pyphare/simulator/simulator.py
- src/simulator/simulator.hpp
- tools/python3/phloping.py
Additional context used
Ruff
pyphare/pyphare/simulator/monitoring.py
6-6:
phlop.dict.ValDictimported but unused; consider usingimportlib.util.find_specto test for availability(F401)
Additional comments not posted (4)
pyphare/pyphare/simulator/monitoring.py (3)
22-22: LGTM!The use of
_globalsfor maintaining state is appropriate and well-implemented.
25-28: LGTM!The function correctly constructs the file path and ensures the directory exists.
31-48: LGTM!The functions correctly manage the monitoring process lifecycle and handle the absence of
phlopgracefully.tools/python3/plotting.py (1)
27-27: Verify the impact of removingsingle_hier_for_all_quantities=True.The removal of this parameter might affect the behavior of the
Runclass. Ensure that this change does not negatively impact data processing or interactions with other components.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (5)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- pyphare/pyphare/simulator/monitoring.py
- pyphare/pyphare/simulator/simulator.py
- src/simulator/simulator.hpp
- tools/python3/phloping.py
Additional comments not posted (1)
tools/python3/plotting.py (1)
27-27: Verify the impact of removingsingle_hier_for_all_quantities=True.The
Runobject is now instantiated with only thediag_dirparameter. Ensure that this change aligns with the expected behavior of theRunclass and does not introduce unintended side effects.
PhilipDeegan
force-pushed
the
statsss
branch
2 times, most recently
from
62412de to
bfe182e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
tools/python3/phloping.py (1)
132-136: Clarify the logic for trimming initial particle counts inLi_timescalculationThe condition
if counts == len(times)and the subsequent slicingself.particles_per_level_per_time_step[ilvl][1:]can be unclear to readers. To enhance code readability, consider adding comments explaining that particle counts may include an initial dump, hence the need to adjustLi_timesaccordingly.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (4 hunks)
- src/hdf5/detail/h5/h5_file.hpp (1 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tests/functional/harris/harris_2d_lb.py (1 hunks)
- tools/python3/phloping.py (1 hunks)
Additional context used
Path-based instructions (2)
src/hdf5/detail/h5/h5_file.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.src/simulator/simulator.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
Additional comments not posted (13)
pyphare/pyphare/simulator/monitoring.py (5)
4-10: LGTM!The function uses the recommended approach of
importlib.util.find_specto check for module availability and correctly handles potential exceptions.
13-19: LGTM!The function correctly uses the
have_phlopfunction to determine the return type and handles the potentialimport-errorwarning.
25-28: LGTM!The function correctly constructs the file path using
pathlib.Pathand ensures the necessary directory structure exists.
31-40: LGTM!The function correctly sets up monitoring using the
AttachableRuntimeStatsManagerand handles the potentialimport-errorwarning. It uses themonitoring_yaml_filefunction to get the file path and starts the stats manager.
43-48: LGTM!The function correctly shuts down the monitoring process by calling the
killmethod on thestats_manif it exists and joining the thread.tests/functional/harris/harris_2d_lb.py (2)
174-176: Verify the correctness of theqtyparameter change.The
qtyparameter for therun.GetB(time).plotmethod has been changed fromf"B{c}"tof"{c}". This change affects the format of the quantity being plotted for each component.Please clarify the intended behavior and ensure that the new format aligns with the expected input for the
run.GetB(time).plotmethod.
179-179: Verify the correctness of theqtyparameter change.The
qtyparameter for therun.GetJ(time).plotmethod has been changed from"Jz"to"z". This change affects the quantity being plotted for the current density.Please clarify the intended behavior and ensure that the new value correctly represents the z-component of the current density as expected by the
run.GetJ(time).plotmethod.pyphare/pyphare/simulator/simulator.py (2)
10-10: LGTM!The import statement is correct and follows the convention of aliasing the module for concise usage.
Line range hint
174-205: Verify the usage of themonitoringparameter in the codebase.The changes to the
runmethod look good! The integration of optional monitoring functionality enhances the capabilities of the simulator without affecting the existing behavior.Please ensure that the
monitoringparameter is properly utilized in the codebase and that thephloplibrary is available when monitoring is enabled.Run the following script to verify the usage of the
monitoringparameter:src/hdf5/detail/h5/h5_file.hpp (1)
55-55: Verify the correctness of thecore::mpi::size()function.The change from
core::mpi_size()tocore::mpi::size()is likely a refactoring and should not introduce any logical errors. However, please ensure that thecore::mpi::size()function is correctly implemented and returns the expected value.Run the following script to verify the correctness of the
core::mpi::size()function:src/simulator/simulator.hpp (2)
370-371: LGTM!The added logging statement is beneficial for tracking the initialization process. It follows the correct format and uses an appropriate log level.
Line range hint
1-1: Verify the impact of removing the logging statement.The logging statement for the
advancemethod has been removed. Please ensure that removing this logging does not hinder the ability to track and debug the advancement process if needed.Consider adding a configuration option to enable/disable logging for the
advancemethod based on the desired verbosity level.tools/python3/phloping.py (1)
143-143: Verify array dimensions and zero values before reshaping and divisionIn the normalization step for higher levels, the code performs:
norm_times.reshape(int(times.shape[0] / substeps), substeps) / Li_times.reshape(Li_times.shape[0], 1)Ensure that the reshaped arrays have compatible dimensions to prevent
ValueErrordue to misaligned shapes. Additionally, verify thatLi_timesdoes not contain zero values to avoid division by zero errors during normalization.
PhilipDeegan
force-pushed
the
statsss
branch
from
bfe182e to
62c04ab
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/phare/phare.hpp (1)
46-46: Improved file path structure for timing logs.The new file path
.phare/timings/rank.{rank}.txtis a good improvement over the previous structure. It provides better organization and clarity for timing log files.Consider adding a check to ensure the
.phare/timings/directory exists before writing the log file. This can prevent potential issues if the directory hasn't been created. You could add this check in the initialization code, for example:#include <filesystem> // ... std::filesystem::create_directories(".phare/timings");This will create the directory if it doesn't exist, ensuring that the log files can be written without issues.
tests/functional/harris/harris_2d_lb.py (2)
18-18: LGTM! Consider adding a comment for clarity.The introduction of the
SCOPE_TIMINGenvironment variable is a good addition for controlling the plotting of run timer data. It follows the existing pattern used forLOAD_BALANCE.Consider adding a brief comment explaining the purpose of this variable, similar to:
# Environment variable to control whether to plot run timer data SCOPE_TIMING = os.getenv("PHARE_SCOPE_TIMING", "True").lower() in ("true", "1", "t")
204-205: LGTM! Consider consistent formatting for multi-line conditions.The addition of the conditional block for plotting run timer data based on the
SCOPE_TIMINGvariable is a good improvement. It provides flexibility in controlling the output of performance data.For consistency with the existing code style, consider formatting the condition on a separate line:
if SCOPE_TIMING: m_plotting.plot_run_timer_data(diag_dir, cpp.mpi_rank())This aligns with the formatting used for the
if __name__ == "__main__":block later in the file.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (4 hunks)
- src/amr/data/field/field_variable_fill_pattern.hpp (2 hunks)
- src/amr/messengers/communicator.hpp (0 hunks)
- src/hdf5/detail/h5/h5_file.hpp (1 hunks)
- src/phare/phare.hpp (1 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tests/functional/harris/harris_2d_lb.py (3 hunks)
- tools/python3/phloping.py (1 hunks)
💤 Files not reviewed due to no reviewable changes (1)
- src/amr/messengers/communicator.hpp
✅ Files skipped from review due to trivial changes (1)
- src/amr/data/field/field_variable_fill_pattern.hpp
🚧 Files skipped from review as they are similar to previous changes (4)
- pyphare/pyphare/simulator/monitoring.py
- pyphare/pyphare/simulator/simulator.py
- src/hdf5/detail/h5/h5_file.hpp
- src/simulator/simulator.hpp
🧰 Additional context used
📓 Path-based instructions (1)
src/phare/phare.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 Ruff
tools/python3/phloping.py
160-160: SyntaxError: Parameter without a default cannot follow a parameter with a default
🔇 Additional comments (4)
tests/functional/harris/harris_2d_lb.py (1)
175-175: Please clarify the changes to theqtyparameters.The modifications to the
qtyparameters in theplotfunction may affect the data being plotted:
- For
run.GetB(time).plot, the change fromf"B{c}"tof"{c}"might alter how the magnetic field components are interpreted or displayed.- For
run.GetJ(time).plot, changing from"Jz"to"z"could affect the current density plot.Could you please explain the reasoning behind these changes and confirm that they don't negatively impact the plot outputs? Additionally, it would be helpful to verify if these changes are consistent with any updates in the underlying plotting functions or data structures.
Also applies to: 180-180
tools/python3/phloping.py (3)
126-136: Improved handling of particle counts for normalizationThe changes in this segment enhance the robustness of the
normalised_times_for_Lmethod. By accounting for potential mismatches between particle counts and time steps, the code now ensures thatLi_timesis correctly aligned with thetimesarray. This improvement helps prevent potential index errors and ensures more accurate normalization results.
150-158: Well-implemented CSV writing functionThe new
write_root_as_csvfunction is a good addition to the codebase. It efficiently uses context managers for file handling and stdout redirection, providing a clean way to write CSV output to a file. The function is well-structured and easy to understand.
Line range hint
1-172: Overall improvements to PHARE scope function timer analysisThe changes in this file enhance the functionality and robustness of the PHARE scope function timer analysis. Key improvements include:
- More robust handling of particle counts in the normalization process.
- Addition of CSV output capabilities with
write_root_as_csvandprint_root_as_csvfunctions.These changes improve the module's ability to handle edge cases and provide more flexible output options. The code is generally well-structured and maintainable.
To further improve the file:
- Address the potential division by zero issue in
normalised_times_for_L.- Fix the syntax error in the
print_root_as_csvfunction signature.After these minor adjustments, the file will be in excellent shape for merging.
🧰 Tools
🪛 Ruff
160-160: SyntaxError: Parameter without a default cannot follow a parameter with a default
| if ilvl == 0: | ||
| return times / self.particles_per_level_per_time_step[0] | ||
| return times / Li_times |
There was a problem hiding this comment.
Prevent potential division by zero
When ilvl == 0, the function returns times / Li_times. This operation could raise a ZeroDivisionError if any element in Li_times is zero.
Consider adding a check to prevent division by zero. For example:
if ilvl == 0:
return np.divide(times, Li_times, out=np.zeros_like(times), where=Li_times!=0)This approach will return 0 for any element where Li_times is 0, preventing the division by zero error.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
tools/python3/phloping.py (1)
152-157: Approve new function with a minor suggestionThe new
write_root_as_csvfunction looks good and properly handles file operations.Consider combining the nested
withstatements for better readability:def write_root_as_csv(scope_timer_file, outfile, headers=None, regex=None): with open(outfile, "w") as f, redirect_stdout(f): print_root_as_csv(scope_timer_file, headers, regex)This change simplifies the code structure without altering its functionality.
🧰 Tools
🪛 Ruff
155-156: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
- pyphare/pyphare/cpp/validate.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- tools/python3/phloping.py (1 hunks)
- tools/python3/plotting.py (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyphare/pyphare/simulator/simulator.py
🧰 Additional context used
🪛 Ruff
tools/python3/phloping.py
155-156: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
171-171: Local variable
dimis assigned to but never usedRemove assignment to unused variable
dim(F841)
🔇 Additional comments (4)
tools/python3/plotting.py (1)
28-28: Approve change and verify consistency across the codebaseThe modification of the timing data file path looks good. It appears to be part of a directory structure reorganization for timing files.
To ensure consistency across the codebase, please run the following script to check for any other occurrences of the old path format:
Additionally, consider updating any relevant documentation that might reference the old file path structure.
✅ Verification successful
Timing data file path change verified
All instances of the old timing file path format have been updated. No remaining occurrences found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for other occurrences of the old timing file path format # Test: Search for the old path format. Expect: No results if all occurrences have been updated. rg --type python '\.phare_times\.\{?rank\}?\.txt'Length of output: 82
Script:
#!/bin/bash # Description: Search for other occurrences of the old timing file path format in Python files # Test: Search for the old path format. Expect: No results if all occurrences have been updated. rg '\.phare_times\.\w+\.txt' --glob '*.py'Length of output: 44
pyphare/pyphare/cpp/validate.py (1)
110-115: Improved directory structure for runtime configuration logging.The changes introduce a more organized approach to storing rank-specific information, which is beneficial for multi-rank MPI environments. The conditional creation of the directory by rank 0 helps prevent potential race conditions.
Consider optimizing the synchronization process:
if cpp_lib.mpi_rank() == 0: rank_info_dir.mkdir(exist_ok=True, parents=True) # Use a file-based approach for synchronization instead of a barrier (rank_info_dir / ".ready").touch() else: while not (rank_info_dir / ".ready").exists(): time.sleep(0.1)This approach could reduce unnecessary waiting time for other ranks.
Please confirm that all ranks have access to the same file system. If this is not guaranteed, additional error handling might be necessary.
This command will help identify other file operations in the codebase, allowing us to verify consistent file system access assumptions.
tools/python3/phloping.py (2)
Line range hint
1-11: Approve removal of unused importThe removal of the unused import for
hierarchy_fromis a good cleanup. This helps keep the import statements relevant and reduces potential confusion.🧰 Tools
🪛 Ruff
155-156: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
171-171: Local variable
dimis assigned to but never usedRemove assignment to unused variable
dim(F841)
Line range hint
1-172: Overall improvements with minor suggestionsThe changes in this file significantly enhance its functionality, particularly in terms of CSV output capabilities and improved particle count handling. The new
write_root_as_csvandprint_root_as_csvfunctions add valuable features for data output.The modifications to the
normalised_times_for_Lmethod improve the handling of particle counts, which should lead to more accurate results.While the overall changes are positive, please address the minor issues mentioned in the previous comments:
- Add a check to prevent division by zero in
normalised_times_for_L.- Combine the nested
withstatements inwrite_root_as_csv.- Fix the function signature and remove the unused variable in
print_root_as_csv.These small adjustments will further improve the code quality and robustness.
🧰 Tools
🪛 Ruff
155-156: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
171-171: Local variable
dimis assigned to but never usedRemove assignment to unused variable
dim(F841)
| Particle counts may include init dump, so be one bigger. | ||
| """ | ||
| times = self.advance_times_for_L(ilvl) | ||
| counts = len(self.particles_per_level_per_time_step[ilvl]) | ||
|
|
||
| # trim init particle count for lvl | ||
| Li_times = ( | ||
| self.particles_per_level_per_time_step[ilvl] | ||
| if counts == len(times) | ||
| else self.particles_per_level_per_time_step[ilvl][1:] | ||
| ) | ||
| if ilvl == 0: | ||
| return times / self.particles_per_level_per_time_step[0] | ||
| return times / Li_times | ||
| substeps = self.steps_per_coarse_timestep_for_L(ilvl) | ||
| norm_times = times.copy() | ||
| return ( | ||
| norm_times.reshape(int(times.shape[0] / substeps), substeps) | ||
| / self.particles_per_level_per_time_step[ilvl].reshape( | ||
| self.particles_per_level_per_time_step[ilvl].shape[0], 1 | ||
| ) | ||
| / Li_times.reshape(Li_times.shape[0], 1) |
There was a problem hiding this comment.
Approve changes with a note on potential division by zero
The changes to handle particle counts and normalization look good. However, there's still a potential issue with division by zero on line 138.
Consider adding a check to prevent division by zero. For example:
if ilvl == 0:
return np.divide(times, Li_times, out=np.zeros_like(times), where=Li_times!=0)This approach will return 0 for any element where Li_times is 0, preventing the division by zero error.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/amr/solvers/solver_ppc.hpp (3)
203-204: LGTM! Consider adding more context to the log message.The addition of logging improves traceability and debugging capabilities. However, consider adding more context to the log message, such as the level number or any relevant parameters.
You could enhance the log message like this:
- PHARE_LOG_SCOPE(1, "SolverPPC::saveState_"); + PHARE_LOG_SCOPE(1, "SolverPPC::saveState_ for level {}", level.getLevelNumber());
227-228: LGTM! Consider adding more context to the log message.The addition of logging improves traceability and debugging capabilities. However, consider adding more context to the log message, such as the level number or any relevant parameters.
You could enhance the log message like this:
- PHARE_LOG_SCOPE(1, "SolverPPC::restoreState_"); + PHARE_LOG_SCOPE(1, "SolverPPC::restoreState_ for level {}", level.getLevelNumber());
Line range hint
449-461: LGTM! Consider adding more context to the log messages for consistency.The addition of logging improves traceability and debugging capabilities. The placement of the logging statements provides good granularity for tracking the progress of the
moveIons_method. However, consider adding more context to the log messages for consistency with other logging statements in the class.You could enhance the log messages like this:
- PHARE_LOG_SCOPE(1, "SolverPPC::moveIons_updatePopulations"); + PHARE_LOG_SCOPE(1, "SolverPPC::moveIons_updatePopulations for level {}", level.getLevelNumber()); - PHARE_LOG_SCOPE(1, "SolverPPC::moveIons_fillIonGhostParticles"); + PHARE_LOG_SCOPE(1, "SolverPPC::moveIons_fillIonGhostParticles for level {}", level.getLevelNumber());
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- src/amr/solvers/solver_ppc.hpp (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/amr/solvers/solver_ppc.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
tools/python3/phloping.py (1)
151-156: LGTM: New write_root_as_csv function with minor optimization suggestionThe new
write_root_as_csvfunction is a good addition, providing a convenient way to write CSV output to a file. The use of context managers ensures proper resource management.Consider combining the nested
withstatements for a slight optimization:def write_root_as_csv(scope_timer_file, outfile, headers=None, regex=None): from contextlib import redirect_stdout with open(outfile, "w") as f, redirect_stdout(f): print_root_as_csv(scope_timer_file, headers, regex)This change reduces indentation and makes the code slightly more concise.
🧰 Tools
🪛 Ruff
154-155: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- tools/python3/phloping.py (2 hunks)
🧰 Additional context used
🪛 Ruff
tools/python3/phloping.py
154-155: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
169-169: Local variable
bitsis assigned to but never usedRemove assignment to unused variable
bits(F841)
🔇 Additional comments (5)
tools/python3/phloping.py (5)
5-11: LGTM: Import statements updated appropriatelyThe changes to the import statements reflect the modifications in the rest of the file. The removal of the
hierarchy_fromimport and the addition ofscope_timerfromphlop.timingare consistent with the updated code structure.
17-17: LGTM: Updated class inheritanceThe change in inheritance from
phScopeTimerFiletost.ScopeTimerFileis consistent with the updated import statements and suggests a refactoring to use the new implementation from thescope_timermodule.
147-149: LGTM: Updated file_parser functionThe change to use
st.file_parseris consistent with the earlier modifications and reflects the refactoring to use the new implementation from thescope_timermodule.
173-181: LGTM: New print_variance_across functionThe new
print_variance_acrossfunction is a well-implemented addition that provides a convenient way to handle command-line arguments for variance calculation. It includes proper error handling, help message display, and is consistent with the rest of the file's changes.
125-142: LGTM with a note on potential division by zeroThe changes to handle particle counts and normalization look good. The new logic correctly addresses the scenario where particle counts may include an initial dump, ensuring proper alignment with the times array.
However, there's still a potential issue with division by zero on line 137. Consider adding a check to prevent this, for example:
if ilvl == 0: return np.divide(times, Li_times, out=np.zeros_like(times), where=Li_times!=0)This approach will return 0 for any element where
Li_timesis 0, preventing the division by zero error.✅ Verification successful
Verified Potential Division by Zero
Only the identified division by zero issue in
tools/python3/phloping.pyat line 137 was found. No other similar patterns exist in the codebase.Please ensure to implement the suggested check to prevent division by zero errors.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if there are any other occurrences of potential division by zero rg --type python 'return\s+\w+\s*/\s*\w+' tools/python3/phloping.pyLength of output: 100
Script:
#!/bin/bash # Verify if there are any other occurrences of potential division by zero rg --glob "*.py" 'return\s+\w+\s*/\s*\w+' tools/python3/phloping.pyLength of output: 105
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
pyphare/pyphare/simulator/simulator.py (5)
5-12: LGTM! Consider grouping imports.The new imports are appropriate for the monitoring functionality being added. However, consider grouping the imports according to PEP 8 guidelines: standard library imports, third-party imports, and local imports.
Here's a suggested regrouping:
import os import datetime import atexit import time as timem from pathlib import Path import numpy as np import pyphare.pharein as ph from . import monitoring as mon
32-34: LGTM! Consider adding error handling.The addition of the directory creation for timing logs is a good implementation. However, consider adding error handling to manage potential exceptions during directory creation.
Here's a suggested improvement:
if SCOPE_TIMING: try: Path(".phare/timings").mkdir(parents=True, exist_ok=True) except PermissionError: print("Warning: Unable to create timing directory due to permission error.")This change ensures the parent directories are created if they don't exist and provides a helpful warning if there's a permission issue.
181-189: LGTM! Consider expanding the docstring.The addition of the
monitoringparameter and its default behavior is well implemented. The docstring provides crucial information about thephlopdependency.Consider expanding the docstring to provide more details:
def run(self, plot_times=False, monitoring=None): """ Run the simulation. Args: plot_times (bool): If True, plot the timestep times. monitoring (bool): If True, enable monitoring. Requires phlop library. If None, uses the PHARE_SIM_MON environment variable. Returns: self: Returns the Simulator instance. """This expanded docstring provides more clarity on the parameters and return value.
192-193: LGTM! Consider adding error handling for monitoring setup.The conditional setup of monitoring is well implemented. However, it would be beneficial to add error handling to manage potential issues during the monitoring setup.
Here's a suggested improvement:
if monitoring: try: mon.setup_monitoring(cpp_lib()) except ImportError: print("Warning: Unable to setup monitoring. Make sure phlop is installed.") monitoring = False except Exception as e: print(f"Warning: Error during monitoring setup: {e}") monitoring = FalseThis change provides more robust error handling and informs the user if there are issues with the monitoring setup.
215-215: Consider making monitoring shutdown conditional.While it's good to ensure monitoring resources are released, calling
monitoring_shutdownunconditionally might not be necessary if monitoring was never set up.Consider modifying the code as follows:
if monitoring: mon.monitoring_shutdown(cpp_lib())This change ensures that the shutdown is only called when monitoring was actually set up, potentially avoiding unnecessary function calls.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/amr/solvers/solver_ppc.hpp (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/amr/solvers/solver_ppc.hpp
🔇 Additional comments (1)
pyphare/pyphare/simulator/simulator.py (1)
16-17: LGTM! Environment variables added as suggested.The new environment variables
PHARE_SIM_MONandPHARE_SCOPE_TIMINGhave been added with default values set to "False", which is consistent with the previous suggestion. This implementation allows for easy enabling of monitoring and timing features when needed.
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
tests/functional/harris/harris_2d_lb.py (3)
18-19: LGTM! Consider default values for production use.The introduction of SCOPE_TIMING and LOAD_BALANCE as environment variables is a good practice for flexible configuration. However, consider setting the default values to False for production use, as timing and load balancing might impact performance.
You could modify the lines as follows:
SCOPE_TIMING = os.getenv("PHARE_SCOPE_TIMING", "False").lower() in ("true", "1", "t") LOAD_BALANCE = os.getenv("LOAD_BALANCE", "False").lower() in ("true", "1", "t")
175-175: Consider using more descriptive labels for clarity.The change to generic labels ("z" instead of "Bz" or "Jz") might make the plots less immediately interpretable. While this could make the function more flexible, it might confuse users unfamiliar with the code.
Consider the following suggestions:
- Add comments explaining the meaning of these generic labels.
- Use more descriptive labels, e.g., "magnetic_field_z" or "current_density_z".
- If flexibility is crucial, consider passing the label as a parameter to the plotting function.
Also applies to: 180-180
204-205: LGTM! Consider adding error handling.The addition of conditional timing data plotting is a good improvement for flexibility. It correctly uses the SCOPE_TIMING variable introduced earlier.
For improved robustness, consider adding error handling:
if SCOPE_TIMING: try: m_plotting.plot_run_timer_data(diag_dir, cpp.mpi_rank()) except Exception as e: print(f"Error plotting timing data: {e}")This will prevent the test from failing if there's an issue with the timing data plotting.
src/hdf5/detail/h5/h5_file.hpp (1)
Line range hint
1-324: Consider future refactoring for improved maintainability.While not directly related to the current change, it's worth noting that this file contains complex C++ code for HDF5 file handling. In future iterations, consider breaking down some of the larger methods (e.g.,
write_attributes_per_mpi) into smaller, more focused functions to improve readability and maintainability.src/amr/solvers/solver_ppc.hpp (2)
219-220: LGTM: Improved logging for better traceability.The addition of the logging statement enhances the traceability of the
saveState_method execution. This is beneficial for debugging and monitoring the application's flow.Consider adding more context to the log message, such as the level number or any relevant parameters, to make the logs even more informative. For example:
PHARE_LOG_SCOPE(1, "SolverPPC::saveState_ for level {}", level.getLevelNumber());
237-238: LGTM: Enhanced logging for improved traceability.The addition of the logging statement improves the traceability of the
restoreState_method execution, which is valuable for debugging and monitoring the application's flow.Consider adding more context to the log message, such as the level number or any relevant parameters, to make the logs even more informative. For example:
PHARE_LOG_SCOPE(1, "SolverPPC::restoreState_ for level {}", level.getLevelNumber());src/simulator/simulator.hpp (2)
370-371: LGTM! Consider adding more context to the log message.The addition of the logging statement improves traceability. However, consider including more context in the log message, such as the current time step or simulation parameters.
You could enhance the log message like this:
- PHARE_LOG_SCOPE(1, "Simulator::initialize"); + PHARE_LOG_SCOPE(1, "Simulator::initialize - startTime: " + std::to_string(startTime_) + ", finalTime: " + std::to_string(finalTime_));
Line range hint
421-421: LGTM! Consider adding more context to the log message.The addition of the logging statement improves traceability. However, consider including more context in the log message, such as the current time step or the
dtparameter.You could enhance the log message like this:
- PHARE_LOG_SCOPE(1, "Simulator::advance"); + PHARE_LOG_SCOPE(1, "Simulator::advance - currentTime: " + std::to_string(currentTime_) + ", dt: " + std::to_string(dt));pyphare/pyphare/simulator/simulator.py (2)
182-182: Enhance the docstring for therunmethodThe current docstring
"monitoring requires phlop"is minimal. To improve code readability and provide clarity to users, consider expanding the docstring to explain the purpose of themonitoringparameter, its default behavior, and how it affects the simulation.For example:
def run(self, plot_times=False, monitoring=None): """ Runs the simulation. Parameters: plot_times (bool): If True, plots the timestep times after the simulation. monitoring (bool): If True, enables resource usage monitoring during the simulation. Defaults to the value of the SIM_MONITOR environment variable. """
Line range hint
192-215: Ensuremonitoring_shutdownis always calledIf an exception occurs during the simulation loop,
mon.monitoring_shutdownmay not be called, potentially leaving monitoring resources improperly closed. To guarantee thatmonitoring_shutdownis executed regardless of exceptions, wrap the simulation loop in atry...finallyblock.Apply this diff to implement the fix:
if monitoring: mon.setup_monitoring(cpp_lib()) perf = [] end_time = self.cpp_sim.endTime() t = self.cpp_sim.currentTime() + try: while t < end_time: tick = timem.time() self.advance() tock = timem.time() ticktock = tock - tick perf.append(ticktock) t = self.cpp_sim.currentTime() if cpp_lib().mpi_rank() == 0: out = f"t = {t:8.5f} - {ticktock:6.5f}sec - total {np.sum(perf):7.4}sec" print(out, end=self.print_eol) print_rank0(f"mean advance time = {np.mean(perf)}") print_rank0(f"total advance time = {datetime.timedelta(seconds=np.sum(perf))}") if plot_times: plot_timestep_time(perf) + finally: + if monitoring: + mon.monitoring_shutdown(cpp_lib()) return self.reset()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
- pyphare/pyphare/cpp/validate.py (1 hunks)
- pyphare/pyphare/simulator/monitoring.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (5 hunks)
- src/amr/solvers/solver_ppc.hpp (2 hunks)
- src/hdf5/detail/h5/h5_file.hpp (1 hunks)
- src/phare/phare.hpp (1 hunks)
- src/simulator/simulator.hpp (1 hunks)
- tests/functional/harris/harris_2d_lb.py (3 hunks)
- tools/python3/phloping.py (2 hunks)
- tools/python3/plotting.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
src/amr/solvers/solver_ppc.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.src/hdf5/detail/h5/h5_file.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.src/phare/phare.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.src/simulator/simulator.hpp (1)
Pattern
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
🪛 Ruff
tools/python3/phloping.py
154-155: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
169-169: Local variable
bitsis assigned to but never usedRemove assignment to unused variable
bits(F841)
🔇 Additional comments (15)
tools/python3/plotting.py (1)
28-28: LGTM! Verify consistency of the new file path across the codebase.The updated file path for timing data is more organized and aligns with the PR objectives. This change improves the structure of the output files.
To ensure consistency, please run the following script to check for any remaining occurrences of the old file path:
If the script returns any results, those occurrences should be updated to use the new file path pattern.
✅ Verification successful
Action Required: Correct the search command to properly target Python files.
Please run the following script to check for any occurrences of the old timing file path pattern:
| rank_info_dir = DOT_PHARE_DIR / "rank_info" | ||
| if cpp_lib.mpi_rank() == 0: | ||
| DOT_PHARE_DIR.mkdir(exist_ok=True, parents=True) | ||
| rank_info_dir.mkdir(exist_ok=True, parents=True) | ||
| cpp_lib.mpi_barrier() | ||
|
|
||
| rank_dir = DOT_PHARE_DIR / f"rank_{cpp_lib.mpi_rank()}" | ||
| rank_dir = rank_info_dir / f"{cpp_lib.mpi_rank()}" |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider moving the MPI barrier for better synchronization
The current placement of the MPI barrier might not fully prevent race conditions. Other ranks could potentially proceed to create their rank-specific directories before rank 0 has finished creating the parent directory.
Consider moving the MPI barrier after the creation of rank_info_dir:
rank_info_dir = DOT_PHARE_DIR / "rank_info"
if cpp_lib.mpi_rank() == 0:
rank_info_dir.mkdir(exist_ok=True, parents=True)
-cpp_lib.mpi_barrier()
+cpp_lib.mpi_barrier() # Move the barrier here
rank_dir = rank_info_dir / f"{cpp_lib.mpi_rank()}"
rank_dir.mkdir(exist_ok=True)This change ensures that all ranks wait until the parent directory is created before proceeding, preventing potential issues in directory creation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| rank_info_dir = DOT_PHARE_DIR / "rank_info" | |
| if cpp_lib.mpi_rank() == 0: | |
| DOT_PHARE_DIR.mkdir(exist_ok=True, parents=True) | |
| rank_info_dir.mkdir(exist_ok=True, parents=True) | |
| cpp_lib.mpi_barrier() | |
| rank_dir = DOT_PHARE_DIR / f"rank_{cpp_lib.mpi_rank()}" | |
| rank_dir = rank_info_dir / f"{cpp_lib.mpi_rank()}" | |
| rank_info_dir = DOT_PHARE_DIR / "rank_info" | |
| if cpp_lib.mpi_rank() == 0: | |
| rank_info_dir.mkdir(exist_ok=True, parents=True) | |
| cpp_lib.mpi_barrier() # Move the barrier here | |
| rank_dir = rank_info_dir / f"{cpp_lib.mpi_rank()}" |
| if _globals.stats_man: | ||
| _globals.stats_man.kill().join() |
There was a problem hiding this comment.
Access _globals['stats_man'] using square brackets
Similar to the previous comment, ensure that you access the stats_man key using square brackets since _globals is a dictionary.
Apply this diff to fix the access:
-if _globals.stats_man:
- _globals.stats_man.kill().join()
+if _globals['stats_man']:
+ _globals['stats_man'].kill().join()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if _globals.stats_man: | |
| _globals.stats_man.kill().join() | |
| if _globals['stats_man']: | |
| _globals['stats_man'].kill().join() |
| return find_spec("phlop.dict") is not None | ||
| except (ImportError, ModuleNotFoundError): | ||
| return False |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Simplify the have_phlop function by removing unnecessary exception handling
The find_spec function returns None if the module is not found and does not raise an exception. The try-except block catching ImportError and ModuleNotFoundError is unnecessary and can be removed for simplicity.
Apply this diff to simplify the function:
def have_phlop():
from importlib.util import find_spec
- try:
- return find_spec("phlop.dict") is not None
- except (ImportError, ModuleNotFoundError):
- return False
+ return find_spec("phlop.dict") is not None📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return find_spec("phlop.dict") is not None | |
| except (ImportError, ModuleNotFoundError): | |
| return False | |
| return find_spec("phlop.dict") is not None |
| if not have_phlop(): | ||
| return dict |
There was a problem hiding this comment.
Return a dictionary instance instead of the dict class
In the valdict function, when have_phlop() is False, return dict returns the dict class itself, not an instance. This can lead to issues when the code expects a dictionary instance. Consider returning a dictionary instance by calling dict(**kwargs) instead.
Apply this diff to fix the issue:
def valdict(**kwargs):
if not have_phlop():
- return dict
+ return dict(**kwargs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if not have_phlop(): | |
| return dict | |
| if not have_phlop(): | |
| return dict(**kwargs) |
| with open(outfile, "w") as f: | ||
| with redirect_stdout(f): |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Combine nested with statements for better readability
Consider combining the nested with statements into a single line to simplify the code and improve readability.
Apply this diff to refactor the code:
-with open(outfile, "w") as f:
- with redirect_stdout(f):
- print_root_as_csv(scope_timer_file, headers, regex)
+with open(outfile, "w") as f, redirect_stdout(f):
+ print_root_as_csv(scope_timer_file, headers, regex)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| with open(outfile, "w") as f: | |
| with redirect_stdout(f): | |
| with open(outfile, "w") as f, redirect_stdout(f): | |
| print_root_as_csv(scope_timer_file, headers, regex) |
🧰 Tools
🪛 Ruff
154-155: Use a single
withstatement with multiple contexts instead of nestedwithstatementsCombine
withstatements(SIM117)
| def print_root_as_csv(scope_timer_file, n_parts, headers=None, regex=None): | ||
| stf = scope_timer_file # alias | ||
| stf = file_parser(stf) if isinstance(stf, str) else stf | ||
|
|
||
| if headers: | ||
| print(",".join(headers)) | ||
| for root in stf.roots: | ||
| s = stf(root.k) | ||
| if regex and regex not in s: | ||
| continue | ||
| bits = s.split(",") | ||
| print(f"{s}{root.t},{root.t/n_parts}") | ||
|
|
There was a problem hiding this comment.
Prevent potential division by zero in print_root_as_csv
The variable n_parts is used as a divisor in root.t / n_parts on line 170. Ensure that n_parts is validated to be a non-zero value to prevent a ZeroDivisionError.
Consider adding a check at the beginning of the function:
def print_root_as_csv(scope_timer_file, n_parts, headers=None, regex=None):
+ if n_parts == 0:
+ raise ValueError("n_parts must be a non-zero value")
stf = scope_timer_file # alias📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def print_root_as_csv(scope_timer_file, n_parts, headers=None, regex=None): | |
| stf = scope_timer_file # alias | |
| stf = file_parser(stf) if isinstance(stf, str) else stf | |
| if headers: | |
| print(",".join(headers)) | |
| for root in stf.roots: | |
| s = stf(root.k) | |
| if regex and regex not in s: | |
| continue | |
| bits = s.split(",") | |
| print(f"{s}{root.t},{root.t/n_parts}") | |
| def print_root_as_csv(scope_timer_file, n_parts, headers=None, regex=None): | |
| if n_parts == 0: | |
| raise ValueError("n_parts must be a non-zero value") | |
| stf = scope_timer_file # alias | |
| stf = file_parser(stf) if isinstance(stf, str) else stf | |
| if headers: | |
| print(",".join(headers)) | |
| for root in stf.roots: | |
| s = stf(root.k) | |
| if regex and regex not in s: | |
| continue | |
| bits = s.split(",") | |
| print(f"{s}{root.t},{root.t/n_parts}") |
🧰 Tools
🪛 Ruff
169-169: Local variable
bitsis assigned to but never usedRemove assignment to unused variable
bits(F841)
| if SCOPE_TIMING: | ||
| Path(".phare/timings").mkdir(exist_ok=True) |
There was a problem hiding this comment.
Add error handling when creating the directory
While creating the .phare/timings directory, exceptions such as permission errors may occur. To ensure the program handles such cases gracefully, consider adding error handling around the directory creation.
Apply this diff to handle potential exceptions:
+try:
if SCOPE_TIMING:
Path(".phare/timings").mkdir(exist_ok=True)
+except Exception as e:
+ print(f"Failed to create directory '.phare/timings': {e}")
+ # Consider additional error handling or logging as needed📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if SCOPE_TIMING: | |
| Path(".phare/timings").mkdir(exist_ok=True) | |
| try: | |
| if SCOPE_TIMING: | |
| Path(".phare/timings").mkdir(exist_ok=True) | |
| except Exception as e: | |
| print(f"Failed to create directory '.phare/timings': {e}") | |
| # Consider additional error handling or logging as needed |
sample for rank 0 harris killed after 20 seconds
rank 0 here is injected so we can add anything else too, like the name/path of the execution script/etc
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Chores