samrai assertions if not in release#862
Conversation
WalkthroughWalkthroughThe recent changes enhance the configurations for the SAMRAI and PHARE projects by incorporating debugging assertions for non-release builds in SAMRAI and modifying the default build type to "Release" for PHARE. These adjustments improve error detection during development and simplify the build process by eliminating unnecessary debug settings, resulting in a more streamlined and efficient workflow. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CMake
participant SAMRAI
Developer->>CMake: Set CMAKE_BUILD_TYPE (not "Release")
CMake->>SAMRAI: Enable DEBUG_OPTIONS
SAMRAI->>SAMRAI: Activate DEBUG_INITIALIZE_UNDEFINED
SAMRAI->>SAMRAI: Activate DEBUG_CHECK_ASSERTIONS
SAMRAI->>SAMRAI: Activate DEBUG_CHECK_DIM_ASSERTIONS
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![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
Outside diff range, codebase verification and nitpick comments (1)
res/cmake/dep/samrai.cmake (1)
27-32: Potential Issue: Unused Debugging OptionsThe debugging options
DEBUG_INITIALIZE_UNDEFINEDandDEBUG_CHECK_ASSERTIONSset inres/cmake/dep/samrai.cmakeare not referenced elsewhere in the codebase. This might indicate incomplete integration or redundancy.
DEBUG_INITIALIZE_UNDEFINEDDEBUG_CHECK_ASSERTIONSPlease verify if these options should be used elsewhere or if they are redundant.
Analysis chain
LGTM! Ensure the debugging options are correctly used.
The code changes are approved.
However, verify that the debugging options
DEBUG_INITIALIZE_UNDEFINED,DEBUG_CHECK_ASSERTIONS, andDEBUG_CHECK_DIM_ASSERTIONSare correctly used in the SAMRAI configuration.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of debugging options in SAMRAI configuration. # Test: Search for the debugging options usage. Expect: Correct usage in SAMRAI configuration. rg --type cmake -A 5 $'DEBUG_INITIALIZE_UNDEFINED|DEBUG_CHECK_ASSERTIONS|DEBUG_CHECK_DIM_ASSERTIONS'Length of output: 700
Script:
#!/bin/bash # Description: Verify the usage of debugging options throughout the entire codebase. # Test: Search for the debugging options usage across all files. Expect: Correct usage in relevant places. rg 'DEBUG_INITIALIZE_UNDEFINED|DEBUG_CHECK_ASSERTIONS|DEBUG_CHECK_DIM_ASSERTIONS'Length of output: 372
Script:
#!/bin/bash # Description: Verify the usage and integration of debugging options in the codebase. # Test: Search for the usage of DEBUG_CHECK_DIM_ASSERTIONS in the header file with context. rg -A 5 'DEBUG_CHECK_DIM_ASSERTIONS' src/amr/data/field/field_variable_fill_pattern.hpp # Test: Search for any references to DEBUG_INITIALIZE_UNDEFINED and DEBUG_CHECK_ASSERTIONS in the codebase. rg 'DEBUG_INITIALIZE_UNDEFINED|DEBUG_CHECK_ASSERTIONS'Length of output: 479
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- CMakeLists.txt (1 hunks)
Additional comments not posted (1)
CMakeLists.txt (1)
5-7: LGTM! The default build type is now set to "Release".This change ensures that the project defaults to a release build, which is suitable for production environments. It simplifies the configuration by removing the previous default of "Debug".
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- pyphare/pyphare/pharesee/particles.py (1 hunks)
- src/amr/load_balancing/load_balancer_details.hpp (1 hunks)
- tests/simulator/test_load_balancing.py (4 hunks)
Files skipped from review due to trivial changes (1)
- src/amr/load_balancing/load_balancer_details.hpp
Additional comments not posted (5)
tests/simulator/test_load_balancing.py (4)
61-61: Verify the impact of simplified density logic.The
densityHighfunction now sets the density arrayrhoto a constant value of1. Ensure this simplification does not negatively impact the simulation's behavior in high-density scenarios.
132-132: Verify the rationale and impact of reduced particle count.The number of particles per cell for the high-density configuration has been changed from
590to90. Ensure this reduction does not negatively impact the simulation's accuracy and performance.
142-142: Verify the rationale and impact of increased particle count.The number of particles per cell for the low-density configuration has been changed from
90to120. Ensure this increase does not negatively impact the simulation's performance.
240-240: Verify the rationale and impact of the changed empirical threshold.The empirical threshold in the assertion has been changed from
0.1to0.15. Ensure this change does not negatively impact the test's sensitivity to variations in standard deviation over time.pyphare/pyphare/pharesee/particles.py (1)
299-300: LGTM! Enhances robustness by preventing errors.The guard clause ensures
particles[key]is not empty before performing aggregation, enhancing the robustness of the function.
* samrai assertions if not in release * default cmake build type to release for distribution * fix lb_test / python dict typo upgrade to C++20 better consting to pick correct generate fn simple Diags in tests - silence hidden overload test test without param compile flag workaround views init field -> grid more more more 🥱 wip updates
* samrai assertions if not in release * default cmake build type to release for distribution * fix lb_test / python dict typo samrai assertions if not in release (PHAREHUB#862) * samrai assertions if not in release * default cmake build type to release for distribution * fix lb_test / python dict typo upgrade to C++20 better consting to pick correct generate fn simple Diags in tests - silence hidden overload test test without param compile flag workaround views init field -> grid more more more 🥱 wip updates
2 participants
Summary by CodeRabbit
New Features
Bug Fixes