system interrogation for mpi/etc

[PhilipDeegan]

This should

1. resolve Caliper undefined reference #749
2. give us a method to check the system before building to see what's available as a sort of "use everything available" cmake setup

This requires updating caliper build on TC to verify, but I've checked locally and it's fine

It can possibly be run from our regular cmake too, and if it was default on, the caliper builds wouldn't need changing

Summary by CodeRabbit

• New Features

  • Introduced a new search functionality with a search bar and results display.
  • Added a new option to enable Position Independent Code during build configuration.
  • Implemented a new build configuration option to enable Interprocedural Optimization by default.

• Enhancements

  • Improved the inclusion of MPI headers across various files to centralize and streamline MPI-related configurations.
  • Updated the build scripts to facilitate easier configuration and building of the project with new options.

• Bug Fixes

  • Fixed an issue with the handling of MPI headers and warning suppression by introducing a new header file that manages MPI inclusion and compiler warnings more effectively.

• Documentation

  • Updated .gitignore to exclude newly generated files from version control.

• Refactor

  • Refactored the MPI header inclusion to use a new centralized header, enhancing maintainability and consistency across the codebase.

[coderabbitai]

Important

Auto Review Skipped

More than 25% of the files skipped due to max files limit. Skipping review to prevent low quality review.

24 files out of 82 files are above the max files limit of 50.

Walkthrough

Walkthrough: The changes across the codebase primarily involve the inclusion of a new header file core/def/mpi.hpp, which conditionally includes _gen_mpi.hpp and wraps the inclusion of mpi.h with specific warning controls. Additionally, a new option withfPIC is introduced in CMake to control the use of Position Independent Code, and the default for withIPO is changed to ON. The build system is also updated with new scripts for configuration and a new CMake project phare_configurator.

Changes:

File(s)	Change Summary
CMakeLists.txt, res/cmake/def.cmake, res/cmake/options.cmake	Introduced withfPIC option and changed withIPO default to ON. Removed setting of CMAKE_POSITION_INDEPENDENT_CODE to ON.
src/core/def/.gitignore	Added new pattern _gen*.
src/core/def/mpi.hpp, src/core/utilities/mpi_utils.hpp, src/initializer/python_data_provider.hpp, src/python3/cpp_simulator.hpp, tests/..., src/amr/..., and various other files	Replaced inclusion of mpi.h with core/def/mpi.hpp and added core/def/mpi.hpp include directive.
tools/config/CMakeLists.txt, tools/config/cmake.sh, tools/config/config.py, tools/config/main.cpp	Added new CMake project phare_configurator, scripts, and Python configuration script for MPI settings.

Assessment against linked issues (Beta):

Objective	Addressed	Explanation
Determine the cause of the undefined reference to symbol _ZN3MPI3Win4FreeEv error when building with the -DwithCaliper flag and SAMRAI pre-installed (749).	❓	The provided changes do not directly address this issue, but the inclusion of a new MPI header file and the changes in the build configuration could potentially impact how MPI symbols are linked. Without specific build logs or error messages, it's unclear if these changes resolve the issue.
Understand why the error does not occur when SAMRAI is included as a subproject (749).	❓	Similar to the above, the changes do not explicitly address this issue. The restructuring of MPI includes and build scripts might influence the build process when SAMRAI is included as a subproject, but there's no direct evidence in the provided diffs that this issue is resolved.

The assessment is inconclusive regarding the linked issue due to the lack of direct evidence in the diffs provided. Further investigation, such as build logs or error messages, would be required to determine if the changes address the issue with Caliper and SAMRAI.

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• If you reply to a review comment from CodeRabbit, the bot will automatically respond.
• To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
• Note: Review comments are made on code diffs or files, not on the PR overview.
• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai help to get help.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• The JSON schema for the configuration file is available here.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

[coderabbitai]

Review Status

Actionable comments generated: 12

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and f7f9693.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (15)

• CMakeLists.txt (1 hunks)
• res/cmake/def.cmake (1 hunks)
• res/cmake/options.cmake (1 hunks)
• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (9)

• res/cmake/def.cmake
• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/initializer/python_data_provider.hpp
• src/python3/cpp_simulator.hpp
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp
• tests/diagnostic/test-diagnostics_2d.cpp
• tools/config/CMakeLists.txt
• tools/config/main.cpp

Additional comments: 4

tests/diagnostic/test-diagnostics_1d.cpp (1)

• 2-2: The inclusion of the MPI header has been centralized through the new core/def/mpi.hpp. This is a good practice as it allows for easier management of system-specific configurations and compiler warnings related to MPI.

CMakeLists.txt (1)

• 17-22: The removal of CMAKE_POSITION_INDEPENDENT_CODE is not shown in the provided hunk. If it was removed, ensure that the new withfPIC option is correctly implemented and that it sets CMAKE_POSITION_INDEPENDENT_CODE accordingly when enabled. This change should be verified across the entire project to ensure shared libraries are still being compiled with the correct flags when necessary.

res/cmake/options.cmake (2)

• 58-58: The default value for withIPO has been changed from OFF to ON. This enables Interprocedural Optimization by default, which can improve performance but may increase build time. Ensure that this change is clearly communicated to developers and that it does not negatively impact the build process for any supported platforms or configurations.

• 62-62: The introduction of the withfPIC option allows for the generation of Position Independent Code. This is useful for creating shared libraries but is not necessary for all types of builds. Verify that this option is well-documented and that developers understand when to enable it.

---start hunk 0---

</blockquote></details></blockquote></details>



</details>

[coderabbitai]

Review Status

Actionable comments generated: 11

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and 54c258b.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (15)

• CMakeLists.txt (1 hunks)
• res/cmake/def.cmake (1 hunks)
• res/cmake/options.cmake (1 hunks)
• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (6)

• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/initializer/python_data_provider.hpp
• tests/diagnostic/test-diagnostics_1d.cpp
• tests/diagnostic/test-diagnostics_2d.cpp
• tools/config/main.cpp

Additional comments: 5

src/core/def/.gitignore (1)

• 1-1: The addition of _gen* to .gitignore is a good practice to avoid committing generated files to the repository. This change will help keep the repository clean from build artifacts or other generated files that match this pattern.

tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1)

• 2-5: The change from direct MPI include to the custom header is a good practice for managing dependencies and ensuring that any necessary preprocessor directives or configurations are centralized. This can help with maintainability and make it easier to handle different build environments or compiler settings.

CMakeLists.txt (1)

• 17-22: The removal of CMAKE_POSITION_INDEPENDENT_CODE setting is not shown in the provided hunk. If this setting was indeed removed, it is important to ensure that shared libraries and dynamic loading still function as expected without the -fPIC flag. This change could affect the ability to create shared libraries on some platforms, as Position Independent Code is often required for such builds.

res/cmake/def.cmake (2)

• 9-11: The conditional block correctly sets the CMAKE_POSITION_INDEPENDENT_CODE based on the withfPIC option. This is a good use of CMake's conditional statements to provide flexible build configurations.

• 13-15: The comment indicates that IPO is disabled if coverage is enabled, but there is no code here that checks for a coverage option. If coverage is a build option, ensure that there is a condition that disables IPO when coverage is enabled to avoid potential conflicts.

[coderabbitai]

Review Status

Actionable comments generated: 6

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and 24ca9c1.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (12)

• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (7)

• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/initializer/python_data_provider.hpp
• src/python3/cpp_simulator.hpp
• tests/diagnostic/test-diagnostics_1d.cpp
• tests/diagnostic/test-diagnostics_2d.cpp
• tools/config/CMakeLists.txt

Additional comments: 5

src/core/def/.gitignore (1)

• 1-1: The change to .gitignore is appropriate for ignoring generated files or directories that match the pattern "_gen*". This is a common practice to keep build or machine-specific files out of version control.

tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1)

• 2-5: The change to include the custom MPI header (core/def/mpi.hpp) instead of directly including mpi.h is a good practice for centralizing the MPI configuration and handling compiler-specific pragmas or warning suppressions. This should help with maintainability and consistency across the codebase.

tools/config/cmake.sh (3)

• 5-5: The error message has been improved based on the previous comment to provide more context to the user. This is a good change as it helps in diagnosing the issue more effectively.

• 10-10: The script now includes a check for the existence of the phare_configurator executable before attempting to run it. This is a good addition as it prevents unclear error messages if the build does not produce the expected output.

• 12-12: The script now includes a check for the availability of python3 before attempting to run the Python script. This is a good practice to ensure that the necessary tools are available before proceeding with the script execution.

[coderabbitai]

Review Status

Actionable comments generated: 6

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and 2b4d424.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (12)

• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (7)

• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/initializer/python_data_provider.hpp
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp
• tests/diagnostic/test-diagnostics_2d.cpp
• tools/config/config.py
• tools/config/main.cpp

Additional comments: 4

src/python3/cpp_simulator.hpp (1)

• 4-10: The change from direct inclusion of mpi.h to core/def/mpi.hpp is a good practice for managing dependencies and ensuring that any necessary preprocessor directives or pragmas are applied consistently. This should help with maintainability and potentially with suppressing warnings from the MPI headers that are not relevant to the project's code.

src/core/def/.gitignore (1)

• 1-1: The change to .gitignore is appropriate for ignoring generated files or directories that match the pattern "_gen*". This will prevent them from being tracked by Git, which is usually desired for build artifacts or other generated files.

tests/diagnostic/test-diagnostics_1d.cpp (1)

• 2-2: The inclusion of the new core/def/mpi.hpp header file is correct and follows the pull request's description to manage MPI-related code centrally. Ensure that the new header file correctly includes mpi.h and manages compiler warnings as intended.

---OVERVIEW---
The changes in this file are limited to the inclusion of the new MPI header file. There are no changes to the logic or functionality of the tests, which implies that the tests should run as before, provided the new header file is correctly implemented.

tools/config/CMakeLists.txt (1)

• 4-4: Ensure that the minimum required CMake version is consistent with the features being used and the project's requirements.

[coderabbitai]

Review Status

Actionable comments generated: 9

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and b7ff09d.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (14)

• src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (1 hunks)
• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/phare_amr.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (8)

• src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp
• src/core/def/.gitignore
• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/phare_amr.hpp
• src/python3/cpp_simulator.hpp
• tests/diagnostic/test-diagnostics_1d.cpp
• tests/diagnostic/test-diagnostics_2d.cpp

Additional comments: 1

tools/config/main.cpp (1)

• 29-40: The main function correctly initializes MPI and handles exceptions that may be thrown by config_mpi. However, it's important to note that MPI_Abort will terminate all processes in the MPI_COMM_WORLD communicator, which is appropriate here since the program cannot recover from the errors being caught. Ensure that this behavior is documented and understood by users of the program.

Overall, the changes in tools/config/main.cpp are well-implemented and follow good C++ practices with exception handling and resource management. Ensure that the new tool and scripts are documented, and their usage is clear to the users.

[coderabbitai]

Review Status

Actionable comments generated: 7

Configuration used: .coderabbit.yaml

Commits

Files that changed from the base of the PR and between cffbb1e and f11a0cd.

Files ignored due to filter (1)

• lgtm.yml

Files selected for processing (52)

• src/amr/data/field/coarsening/default_field_coarsener.hpp (1 hunks)
• src/amr/data/field/coarsening/field_coarsen_index_weight.hpp (1 hunks)
• src/amr/data/field/coarsening/field_coarsen_operator.hpp (1 hunks)
• src/amr/data/field/coarsening/magnetic_field_coarsener.hpp (1 hunks)
• src/amr/data/field/field_data.hpp (2 hunks)
• src/amr/data/field/field_data_factory.hpp (1 hunks)
• src/amr/data/field/field_geometry.hpp (1 hunks)
• src/amr/data/field/field_overlap.hpp (1 hunks)
• src/amr/data/field/field_variable.hpp (1 hunks)
• src/amr/data/field/field_variable_fill_pattern.hpp (2 hunks)
• src/amr/data/field/refine/electric_field_refiner.hpp (1 hunks)
• src/amr/data/field/refine/field_linear_refine.hpp (1 hunks)
• src/amr/data/field/refine/field_refine_operator.hpp (1 hunks)
• src/amr/data/field/refine/field_refiner.hpp (1 hunks)
• src/amr/data/field/refine/linear_weighter.hpp (1 hunks)
• src/amr/data/field/refine/magnetic_field_refiner.hpp (1 hunks)
• src/amr/data/field/time_interpolate/field_linear_time_interpolate.hpp (1 hunks)
• src/amr/data/particles/particles_data.hpp (1 hunks)
• src/amr/data/particles/particles_data_factory.hpp (1 hunks)
• src/amr/data/particles/particles_variable.hpp (1 hunks)
• src/amr/data/particles/refine/particles_data_split.hpp (1 hunks)
• src/amr/messengers/communicator.hpp (1 hunks)
• src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (1 hunks)
• src/amr/messengers/hybrid_messenger_strategy.hpp (1 hunks)
• src/amr/messengers/messenger.hpp (1 hunks)
• src/amr/messengers/mhd_messenger.hpp (1 hunks)
• src/amr/multiphysics_integrator.hpp (1 hunks)
• src/amr/physical_models/mhd_model.hpp (1 hunks)
• src/amr/resources_manager/amr_utils.hpp (1 hunks)
• src/amr/resources_manager/resources_guards.hpp (2 hunks)
• src/amr/resources_manager/resources_manager.hpp (1 hunks)
• src/amr/solvers/solver.hpp (2 hunks)
• src/amr/solvers/solver_ppc.hpp (1 hunks)
• src/amr/tagging/hybrid_tagger.hpp (1 hunks)
• src/amr/types/amr_types.hpp (1 hunks)
• src/amr/utilities/box/amr_box.hpp (1 hunks)
• src/amr/wrappers/hierarchy.hpp (1 hunks)
• src/amr/wrappers/integrator.hpp (1 hunks)
• src/core/def/.gitignore (1 hunks)
• src/core/def/mpi.hpp (1 hunks)
• src/core/utilities/mpi_utils.hpp (1 hunks)
• src/initializer/python_data_provider.hpp (1 hunks)
• src/phare_amr.hpp (1 hunks)
• src/python3/cpp_simulator.hpp (1 hunks)
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp (1 hunks)
• tests/amr/tagging/test_tagging.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_1d.cpp (1 hunks)
• tests/diagnostic/test-diagnostics_2d.cpp (1 hunks)
• tools/config/CMakeLists.txt (1 hunks)
• tools/config/cmake.sh (1 hunks)
• tools/config/config.py (1 hunks)
• tools/config/main.cpp (1 hunks)

Files skipped from review due to trivial changes (39)

• src/amr/data/field/coarsening/default_field_coarsener.hpp
• src/amr/data/field/coarsening/field_coarsen_index_weight.hpp
• src/amr/data/field/coarsening/magnetic_field_coarsener.hpp
• src/amr/data/field/field_data.hpp
• src/amr/data/field/field_data_factory.hpp
• src/amr/data/field/field_geometry.hpp
• src/amr/data/field/field_overlap.hpp
• src/amr/data/field/field_variable.hpp
• src/amr/data/field/field_variable_fill_pattern.hpp
• src/amr/data/field/refine/electric_field_refiner.hpp
• src/amr/data/field/refine/field_linear_refine.hpp
• src/amr/data/field/refine/field_refine_operator.hpp
• src/amr/data/field/refine/field_refiner.hpp
• src/amr/data/field/refine/magnetic_field_refiner.hpp
• src/amr/data/field/time_interpolate/field_linear_time_interpolate.hpp
• src/amr/data/particles/particles_data.hpp
• src/amr/data/particles/particles_variable.hpp
• src/amr/data/particles/refine/particles_data_split.hpp
• src/amr/messengers/communicator.hpp
• src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp
• src/amr/messengers/hybrid_messenger_strategy.hpp
• src/amr/messengers/messenger.hpp
• src/amr/physical_models/mhd_model.hpp
• src/amr/resources_manager/amr_utils.hpp
• src/amr/resources_manager/resources_manager.hpp
• src/amr/solvers/solver.hpp
• src/amr/solvers/solver_ppc.hpp
• src/amr/tagging/hybrid_tagger.hpp
• src/amr/types/amr_types.hpp
• src/amr/utilities/box/amr_box.hpp
• src/amr/wrappers/hierarchy.hpp
• src/amr/wrappers/integrator.hpp
• src/core/def/mpi.hpp
• src/core/utilities/mpi_utils.hpp
• src/phare_amr.hpp
• tests/amr/multiphysics_integrator/test_multiphysics_integrator.cpp
• tests/amr/tagging/test_tagging.cpp
• tests/diagnostic/test-diagnostics_1d.cpp
• tests/diagnostic/test-diagnostics_2d.cpp

Additional comments: 12

src/amr/messengers/mhd_messenger.hpp (1)

• 4-11: The inclusion of the new core/def/mpi.hpp header file is a good practice to manage MPI-related functionality and compiler warnings in a centralized manner. This change should be checked for consistency across all files where mpi.h was previously included directly.

src/initializer/python_data_provider.hpp (1)

• 10-10: The change from "pragma_disable.hpp" to "core/def/pragma_disable.hpp" indicates a reorganization of the include paths, which is a common task when improving project structure. Ensure that the new path is correctly referenced throughout the project and that the old path is no longer used anywhere else.

src/core/def/.gitignore (1)

• 1-1: The addition to .gitignore is appropriate for ignoring generated files that match the pattern _gen*. This will prevent any files or directories starting with _gen from being tracked by Git, which is useful for build artifacts or other generated files that should not be part of the version control history.

src/python3/cpp_simulator.hpp (1)

• 4-10: The change from direct inclusion of mpi.h to core/def/mpi.hpp is a good practice for managing dependencies and ensuring that any necessary preprocessor directives or pragmas are applied consistently. This should help with maintainability and potentially with suppressing warnings from the MPI headers that are not relevant to the project's code.

src/amr/data/field/refine/linear_weighter.hpp (1)

• 5-5: The inclusion of the new core/def/mpi.hpp header file is a project-wide change for handling MPI more robustly. This change is part of a larger effort to improve system interrogation for MPI and should be verified to ensure that it does not introduce any conflicts or issues with existing MPI functionality in the project.

src/amr/data/field/coarsening/field_coarsen_operator.hpp (1)

• 5-5: The inclusion of the new core/def/mpi.hpp header file is a significant change as it may alter the behavior of the MPI calls within this file due to the wrapping of the standard mpi.h with pragmas to disable and then re-enable specific compiler warnings. This should be thoroughly tested to ensure that it does not introduce any unexpected behavior or side effects.

src/amr/data/particles/particles_data_factory.hpp (1)

• 4-10: The inclusion of the new core/def/mpi.hpp header file is a good practice to ensure that MPI-related code is consistent across the project. This change also helps in managing compiler warnings specific to MPI which can be controlled centrally from the new header file. Make sure that the inclusion of this header does not introduce any conflicts or redefine any macros that could affect the compilation or runtime behavior of the project.

src/amr/resources_manager/resources_guards.hpp (1)

• 69-73: ```
  The changes to the ResourcesGuard class are significant in terms of object lifecycle management. By deleting the default and copy constructors and making the class only movable, it ensures that resources are not accidentally copied, which could lead to issues with resource management in an MPI context. This is a good practice for classes that manage resources in a concurrent environment. However, it's important to ensure that all uses of `ResourcesGuard` within the codebase are updated to work with these changes, as they can no longer be copied, only moved.

---end hunk 1---

tools/config/CMakeLists.txt (3)

• 17-17: The previous comment about using the variables provided by find_package(MPI) is still valid. Replace mpi and mpi_cxx with the variables provided by CMake's find_package(MPI).

- target_link_libraries(${PROJECT_NAME} mpi mpi_cxx)
+ target_link_libraries(${PROJECT_NAME} ${MPI_C_LIBRARIES} ${MPI_CXX_LIBRARIES})

• 18-18: The previous comment about preferring target_link_libraries with absolute library paths over target_link_directories is still valid. This change should be made to ensure that the correct libraries are linked and to avoid potential issues with linker search paths.

• 20-25: The commented-out code for checking available CMake variables should be removed if it is not needed. Keeping the CMake files clean and maintainable is important, and commented-out code can lead to confusion and clutter.

src/amr/multiphysics_integrator.hpp (1)

• 13-17: The inclusion of the new core/def/mpi.hpp header file is a significant change. It is important to ensure that this new header file is properly tested and that it does not introduce any conflicts or issues with existing MPI code. The use of pragmas to disable and re-enable compiler warnings around the mpi.h inclusion is a good practice to prevent warnings from external headers from polluting the project's build output. However, it is crucial to verify that the specific warnings being suppressed are well understood and that suppressing them does not hide any potential issues in the MPI code.

Overall, the file multiphysics_integrator.hpp seems to be untouched except for the addition of the new MPI header. This suggests that the primary goal was to encapsulate MPI-related includes and warnings within a single header file, which is a good practice for maintainability and clarity. It is recommended to perform a full build and run all relevant tests to ensure that the introduction of this header has not affected the behavior of the MPI-related code.

[nicolasaunai]

Ok I added few comments we mentioned this morning.
Could you also add some comment in the tools/config/ directory, maybe some readme or something, that just briefly explain the intent behind these files.