[SPH] Add a perfectly load balanced particle setup

[tdavidcl]

No description provided.

[tdavidcl]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a new, more sophisticated particle setup implementation (apply_setup_new) designed for better load balancing, especially in large-scale simulations. This new method provides fine-grained control over particle generation and communication, and includes an option for detailed logging of the setup process.

To demonstrate this new functionality, a comprehensive example script (run_sphsetup_logs.py) has been added. This script not only shows how to use the new setup function but also includes utilities for visualizing the setup logs, which is a great addition for debugging and analysis.

My review focuses on improving the new example script's robustness and maintainability, and addressing some potential issues in the C++ implementation. Key feedback includes:

• Using standard library constants and functions for better precision and portability in the Python example.
• Correcting a misleading warning in the Python bindings.
• Improving code quality in the C++ implementation by removing a hardcoded filename and suggesting a refactor of a very large function.
• A request for clarification on a magic number change in a benchmark file.

All original comments have been retained as none contradicted the provided rules, and no modifications were necessary based on the specified guidelines.

[github-actions]

Workflow report

workflow report corresponding to commit 2760f3e
Commiter email is timothee.davidcleris@proton.me
GitHub page artifact URL GitHub page artifact link (can expire)

Pre-commit check report

Pre-commit check: ✅

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for merge conflicts................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
check for added large files..............................................Passed
check for case conflicts.................................................Passed
check for broken symlinks................................................Passed
check yaml...............................................................Passed
detect private key.......................................................Passed
No-tabs checker..........................................................Passed
Tabs remover.............................................................Passed
Validate GitHub Workflows................................................Passed
clang-format.............................................................Passed
black....................................................................Passed
ruff check...............................................................Passed
Check doxygen headers....................................................Passed
Check license headers....................................................Passed
Check #pragma once.......................................................Passed
Check SYCL #include......................................................Passed
No ssh in git submodules remote..........................................Passed

Test pipeline can run.

Clang-tidy diff report

No relevant changes found.
Well done!

You should now go back to your normal life and enjoy a hopefully sunny day while waiting for the review.

Doxygen diff with main

Removed warnings : 14
New warnings : 27
Warnings count : 7595 → 7608 (0.2%)

Detailed changes :

+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:52: warning: Member apply_setup_new(SetupNodePtr setup, bool part_reordering, std::optional< u32 > gen_count_per_step=std::nullopt, std::optional< u32 > insert_count_per_step=std::nullopt, std::optional< u64 > max_msg_count_per_rank_per_step=std::nullopt, std::optional< u64 > max_data_count_per_rank_per_step=std::nullopt, bool do_setup_log=false) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:52: warning: Member make_generator_lattice_hcp(Tscal dr, std::pair< Tvec, Tvec > box) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:55: warning: Member make_generator_disc_mc(Tscal part_mass, Tscal disc_mass, Tscal r_in, Tscal r_out, std::function< Tscal(Tscal)> sigma_profile, std::function< Tscal(Tscal)> H_profile, std::function< Tscal(Tscal)> rot_profile, std::function< Tscal(Tscal)> cs_profile, std::mt19937 eng, Tscal init_h_factor) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:61: warning: Member make_generator_lattice_hcp(Tscal dr, std::pair< Tvec, Tvec > box) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:64: warning: Member make_generator_disc_mc(Tscal part_mass, Tscal disc_mass, Tscal r_in, Tscal r_out, std::function< Tscal(Tscal)> sigma_profile, std::function< Tscal(Tscal)> H_profile, std::function< Tscal(Tscal)> rot_profile, std::function< Tscal(Tscal)> cs_profile, std::mt19937 eng, Tscal init_h_factor) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:67: warning: Member make_combiner_add(SetupNodePtr parent1, SetupNodePtr parent2) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:70: warning: Member make_modifier_warp_disc(SetupNodePtr parent, Tscal Rwarp, Tscal Hwarp, Tscal inclination, Tscal posangle) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:73: warning: Member make_modifier_custom_warp(SetupNodePtr parent, std::function< Tscal(Tscal)> inc_profile, std::function< Tscal(Tscal)> psi_profile, std::function< Tvec(Tscal)> k_profile) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:76: warning: Member make_combiner_add(SetupNodePtr parent1, SetupNodePtr parent2) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:79: warning: Member make_modifier_add_offset(SetupNodePtr parent, Tvec offset_postion, Tvec offset_velocity) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:79: warning: Member make_modifier_warp_disc(SetupNodePtr parent, Tscal Rwarp, Tscal Hwarp, Tscal inclination, Tscal posangle) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:82: warning: Member make_modifier_custom_warp(SetupNodePtr parent, std::function< Tscal(Tscal)> inc_profile, std::function< Tscal(Tscal)> psi_profile, std::function< Tvec(Tscal)> k_profile) (function) of class shammodels::sph::modules::SPHSetup is not documented.
- src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:82: warning: Member make_modifier_filter(SetupNodePtr parent, std::function< bool(Tvec)> filter) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:88: warning: Member make_modifier_add_offset(SetupNodePtr parent, Tvec offset_postion, Tvec offset_velocity) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/include/shammodels/sph/modules/SPHSetup.hpp:91: warning: Member make_modifier_filter(SetupNodePtr parent, std::function< bool(Tvec)> filter) (function) of class shammodels::sph::modules::SPHSetup is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:186: warning: Compound SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:187: warning: Compound SetupLog::State is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:188: warning: Member count_per_rank (variable) of struct SetupLog::State is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:189: warning: Member msg_list (variable) of struct SetupLog::State is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:190: warning: Member state (variable) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:192: warning: Member step_counter (variable) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:194: warning: Member json_data (variable) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:196: warning: Member log_state() (function) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:204: warning: Member dump_state() (function) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:217: warning: Member update_count_per_rank(u64 count) (function) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:227: warning: Member update_msg_list(std::vector< std::tuple< u32, u32, u64 > > &msg_list) (function) of struct SetupLog is not documented.
+ src/shammodels/sph/src/modules/SPHSetup.cpp:235: warning: Member golden_number (variable) of file SPHSetup.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1004: warning: Member add_analysisEnergyKinetic_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1020: warning: Member add_analysisEnergyPotential_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1027: warning: Member add_analysisBarycenter_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1036: warning: Member add_analysisTotalMomentum_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1045: warning: Member add_analysisEnergyKinetic_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1054: warning: Member analysis_impl(shammodels::sph::Model< Tvec, SPHKernel > &model) -> Analysis (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1059: warning: Member register_analysis_impl_for_each_kernel(py::module &msph, const char *name_class) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1061: warning: Member add_analysisEnergyPotential_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1077: warning: Member add_analysisTotalMomentum_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1095: warning: Member analysis_impl(shammodels::sph::Model< Tvec, SPHKernel > &model) -> Analysis (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1100: warning: Member register_analysis_impl_for_each_kernel(py::module &msph, const char *name_class) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:1120: warning: Member Register_pymod(pysphmodel) (function) of file pySPHModel.cpp is not documented.
+ src/shammodels/sph/src/pySPHModel.cpp:1161: warning: Member Register_pymod(pysphmodel) (function) of file pySPHModel.cpp is not documented.
- src/shammodels/sph/src/pySPHModel.cpp:986: warning: Member add_analysisBarycenter_instance(py::module &m, std::string name_model) (function) of file pySPHModel.cpp is not documented.