introduce charge, mass and particle densities #1041
Conversation
📝 WalkthroughWalkthroughThis change replaces the use of "density" (number density) with "charge_density" (charge density) throughout the codebase for ions and their diagnostics. It introduces explicit separation between particle density and charge density in C++ core classes, updates Python and test code to request and process "charge_density", and adds supporting utilities and tests for these new conventions. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant PythonSim
participant CppCore
participant Diagnostics
participant HDF5
participant TestSuite
User->>PythonSim: Configure simulation with FluidDiagnostics("charge_density")
PythonSim->>CppCore: Initialize ion populations
CppCore->>CppCore: Compute particleDensity and chargeDensity for each population
CppCore->>Diagnostics: Write "charge_density" and "mass_density" to HDF5
Diagnostics->>HDF5: Store charge_density datasets
TestSuite->>PythonSim: Load simulation results
PythonSim->>Diagnostics: Read "charge_density" from HDF5
TestSuite->>TestSuite: Compare to analytical charge_density profiles
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| return compute_hier_from(compute_, hier, **kwargs) | ||
|
|
||
|
|
||
| def hierarchy_from_func(func, hier, **kwargs): |
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns
|
|
||
| def hierarchy_from_func1d(func, hier, **kwargs): | ||
| assert hier.ndim == 1 | ||
|
|
Check notice
Code scanning / CodeQL
Explicit returns mixed with implicit (fall through) returns Note
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/core/data/ions/ions.hpp (1)
111-144: Add explicit zero-density guards incomputeBulkVelocityto avoid division by zeroThe current code unconditionally divides the accumulated flux by
massDensity_, which may be zero in some cells, leading to NaNs or infinities and breaking physical consistency. You need to handle zero-density cases per cell, for example by defaulting the bulk velocity to zero (or another physically appropriate value) when density is zero.Please update
src/core/data/ions/ions.hppincomputeBulkVelocity()(around lines 138–143) like so:- std::transform(std::begin(vx), std::end(vx), std::begin(massDensity_), std::begin(vx), - std::divides<Float>{}); + std::transform(std::begin(vx), std::end(vx), std::begin(massDensity_), std::begin(vx), + [](Float fluxVal, Float rho) { + return rho != 0 ? fluxVal / rho : Float{0}; + }); - std::transform(std::begin(vy), std::end(vy), std::begin(massDensity_), std::begin(vy), - std::divides<Float>{}); + std::transform(std::begin(vy), std::end(vy), std::begin(massDensity_), std::begin(vy), + [](Float fluxVal, Float rho) { + return rho != 0 ? fluxVal / rho : Float{0}; + }); - std::transform(std::begin(vz), std::end(vz), std::begin(massDensity_), std::begin(vz), - std::divides<Float>{}); + std::transform(std::begin(vz), std::end(vz), std::begin(massDensity_), std::begin(vz), + [](Float fluxVal, Float rho) { + return rho != 0 ? fluxVal / rho : Float{0}; + });This ensures zero-density cells produce well-defined bulk velocities in line with the physics requirements.
🧹 Nitpick comments (4)
src/amr/tagging/default_hybrid_tagger_strategy.hpp (1)
39-39: API change is correct, but consider the unused variable.The change from
model.state.ions.density()tomodel.state.ions.chargeDensity()correctly reflects the API refactoring. However, the variableNis declared but never used in the tagging logic. Consider whether this variable should be removed or if density-based tagging criteria are planned for future implementation.tests/functional/harris/harris_2d_lb.py (2)
75-75: Improve variable naming for clarity.The parameter name
lis ambiguous and could be confused with the number1.- def S(y, y0, l): - return 0.5 * (1.0 + np.tanh((y - y0) / l)) + def S(y, y0, width): + return 0.5 * (1.0 + np.tanh((y - y0) / width))
189-189: Modernize super() calls for Python 3.Use the more concise Python 3 style super() without arguments.
- super(HarrisTest, self).__init__(*args, **kwargs) + super().__init__(*args, **kwargs)- super(HarrisTest, self).tearDown() + super().tearDown()Also applies to: 193-193
tests/simulator/initialize/density_check.py (1)
279-279: Remove leftover debug comment.Remove the file path comment that appears to be debug information:
- # /home/smets/codes/far/PHARE/tests/simulator/initialize - if __name__ == "__main__":
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
.gitignore(1 hunks)pyphare/pyphare/pharein/diagnostics.py(1 hunks)pyphare/pyphare/pharesee/hierarchy/__init__.py(2 hunks)pyphare/pyphare/pharesee/hierarchy/fromfunc.py(1 hunks)pyphare/pyphare/pharesee/hierarchy/hierarchy.py(1 hunks)pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py(1 hunks)pyphare/pyphare/pharesee/plotting.py(1 hunks)pyphare/pyphare/pharesee/run/run.py(3 hunks)pyphare/pyphare_tests/test_pharesee/test_hierarchy.py(1 hunks)src/amr/level_initializer/hybrid_level_initializer.hpp(1 hunks)src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp(2 hunks)src/amr/physical_models/hybrid_model.hpp(1 hunks)src/amr/tagging/default_hybrid_tagger_strategy.hpp(1 hunks)src/core/data/electrons/electrons.hpp(4 hunks)src/core/data/ions/ion_population/ion_population.hpp(5 hunks)src/core/data/ions/ions.hpp(9 hunks)src/core/numerics/interpolator/interpolator.hpp(3 hunks)src/core/numerics/ion_updater/ion_updater.hpp(4 hunks)src/core/numerics/moments/moments.hpp(2 hunks)src/diagnostic/detail/types/fluid.hpp(5 hunks)src/phare/phare_init.py(1 hunks)src/phare/phare_init_small.py(1 hunks)src/python3/patch_level.hpp(2 hunks)tests/core/data/electrons/test_electrons.cpp(8 hunks)tests/core/data/ion_population/test_ion_population_fixtures.hpp(1 hunks)tests/core/data/ions/test_ions.cpp(1 hunks)tests/core/numerics/interpolator/test_main.cpp(15 hunks)tests/core/numerics/ion_updater/test_updater.cpp(12 hunks)tests/diagnostic/__init__.py(1 hunks)tests/diagnostic/test_diagnostics.hpp(4 hunks)tests/diagnostic/test_diagnostics.ipp(1 hunks)tests/functional/alfven_wave/alfven_wave1d.py(1 hunks)tests/functional/dispersion/dispersion.py(1 hunks)tests/functional/harris/harris_2d_lb.py(1 hunks)tests/functional/shock/shock.py(1 hunks)tests/functional/td/td1d.py(1 hunks)tests/functional/tdtagged/td1dtagged.py(1 hunks)tests/functional/translation/translat1d.py(2 hunks)tests/simulator/initialize/CMakeLists.txt(1 hunks)tests/simulator/initialize/density_check.py(1 hunks)tests/simulator/refinement/test_2d_10_core.py(1 hunks)tests/simulator/refinement/test_2d_2_core.py(1 hunks)tests/simulator/test_advance.py(3 hunks)tests/simulator/test_initialization.py(2 hunks)tests/simulator/test_run.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.hpp`: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
**/*.hpp: Review the C++ code, point out issues relative to principles of clean code, expressiveness, and performance.
⚙️ Source: CodeRabbit Configuration File
List of files the instruction was applied to:
src/amr/level_initializer/hybrid_level_initializer.hppsrc/amr/tagging/default_hybrid_tagger_strategy.hppsrc/python3/patch_level.hppsrc/amr/physical_models/hybrid_model.hpptests/core/data/ion_population/test_ion_population_fixtures.hppsrc/core/numerics/ion_updater/ion_updater.hppsrc/diagnostic/detail/types/fluid.hppsrc/amr/messengers/hybrid_hybrid_messenger_strategy.hpptests/diagnostic/test_diagnostics.hppsrc/core/numerics/moments/moments.hppsrc/core/data/electrons/electrons.hppsrc/core/numerics/interpolator/interpolator.hppsrc/core/data/ions/ion_population/ion_population.hppsrc/core/data/ions/ions.hpp
🧠 Learnings (32)
📓 Common learnings
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#888
File: src/hdf5/detail/h5/h5_file.hpp:78-79
Timestamp: 2024-10-09T08:32:15.667Z
Learning: Avoid commenting on draft PRs for the user PhilipDeegan.
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#888
File: src/hdf5/detail/h5/h5_file.hpp:78-79
Timestamp: 2024-09-05T17:02:58.784Z
Learning: Avoid commenting on draft PRs for the user PhilipDeegan.
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#888
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:566-568
Timestamp: 2024-09-09T13:57:02.285Z
Learning: Avoid commenting on draft PRs for PhilipDeegan unless explicitly requested.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#784
File: tests/simulator/test_restarts.py:333-339
Timestamp: 2024-07-26T22:04:34.160Z
Learning: PhilipDeegan has indicated a preference for minimal and efficient code, even after making changes to get a function working. There may be opportunities to remove or optimize parts of the code such as sleep/wait/advances.
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#784
File: tests/simulator/test_restarts.py:333-339
Timestamp: 2024-10-09T08:32:15.667Z
Learning: PhilipDeegan has indicated a preference for minimal and efficient code, even after making changes to get a function working. There may be opportunities to remove or optimize parts of the code such as sleep/wait/advances.
src/phare/phare_init_small.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/functional/td/td1d.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/simulator/test_run.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/amr/level_initializer/hybrid_level_initializer.hpp (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/functional/tdtagged/td1dtagged.py (3)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE simulations, velocity calculations must account for per-population densities that can be strictly zero in given cells. Simple zero-return solutions for division by zero are inadequate as they don't reflect the correct physics.
src/phare/phare_init.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/functional/translation/translat1d.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/simulator/initialize/CMakeLists.txt (2)
Learnt from: nicolasaunai
PR: PHAREHUB/PHARE#591
File: res/cmake/options.cmake:73-75
Timestamp: 2025-02-07T14:35:14.630Z
Learning: In CMake configuration for PHARE, `lowResourceTests` and `highResourceTests` options must be mutually exclusive and cannot be enabled simultaneously.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/simulator/refinement/test_2d_10_core.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (4)
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#910
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:7-7
Timestamp: 2024-10-18T13:23:32.074Z
Learning: In the `pyphare.pharesee.hierarchy` module, importing `PatchHierarchy` and `format_timestamp` from `hierarchy.py` into `hierarchy_utils.py` is acceptable as long as `hierarchy.py` does not import `hierarchy_utils.py`, thereby avoiding a cyclic import.
Learnt from: nicolasaunai
PR: PHAREHUB/PHARE#891
File: tests/simulator/test_advance.py:50-50
Timestamp: 2024-09-19T17:16:57.787Z
Learning: In the `getHierarchy()` function, use random initialization for `model_init` by default instead of setting a fixed seed.
Learnt from: nicolasaunai
PR: PHAREHUB/PHARE#891
File: tests/simulator/test_advance.py:50-50
Timestamp: 2024-10-09T08:32:15.667Z
Learning: In the `getHierarchy()` function, use random initialization for `model_init` by default instead of setting a fixed seed.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/amr/tagging/default_hybrid_tagger_strategy.hpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/functional/dispersion/dispersion.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/simulator/refinement/test_2d_2_core.py (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/amr/physical_models/hybrid_model.hpp (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/simulator/test_initialization.py (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/diagnostic/__init__.py (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/core/numerics/ion_updater/ion_updater.hpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/diagnostic/detail/types/fluid.hpp (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
pyphare/pyphare/pharesee/hierarchy/__init__.py (3)
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#910
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:7-7
Timestamp: 2024-10-18T13:23:32.074Z
Learning: In the `pyphare.pharesee.hierarchy` module, importing `PatchHierarchy` and `format_timestamp` from `hierarchy.py` into `hierarchy_utils.py` is acceptable as long as `hierarchy.py` does not import `hierarchy_utils.py`, thereby avoiding a cyclic import.
Learnt from: nicolasaunai
PR: PHAREHUB/PHARE#891
File: tests/simulator/test_advance.py:50-50
Timestamp: 2024-09-19T17:16:57.787Z
Learning: In the `getHierarchy()` function, use random initialization for `model_init` by default instead of setting a fixed seed.
Learnt from: nicolasaunai
PR: PHAREHUB/PHARE#891
File: tests/simulator/test_advance.py:50-50
Timestamp: 2024-10-09T08:32:15.667Z
Learning: In the `getHierarchy()` function, use random initialization for `model_init` by default instead of setting a fixed seed.
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1)
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#910
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:7-7
Timestamp: 2024-10-18T13:23:32.074Z
Learning: In the `pyphare.pharesee.hierarchy` module, importing `PatchHierarchy` and `format_timestamp` from `hierarchy.py` into `hierarchy_utils.py` is acceptable as long as `hierarchy.py` does not import `hierarchy_utils.py`, thereby avoiding a cyclic import.
src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
tests/diagnostic/test_diagnostics.hpp (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
pyphare/pyphare/pharesee/run/run.py (1)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
src/core/numerics/moments/moments.hpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/core/data/electrons/electrons.hpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
src/core/data/ions/ion_population/ion_population.hpp (3)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE simulations, velocity calculations must account for per-population densities that can be strictly zero in given cells. Simple zero-return solutions for division by zero are inadequate as they don't reflect the correct physics.
tests/core/numerics/ion_updater/test_updater.cpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
pyphare/pyphare/pharesee/hierarchy/fromfunc.py (1)
Learnt from: PhilipDeegan
PR: PHAREHUB/PHARE#910
File: pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py:7-7
Timestamp: 2024-10-18T13:23:32.074Z
Learning: In the `pyphare.pharesee.hierarchy` module, importing `PatchHierarchy` and `format_timestamp` from `hierarchy.py` into `hierarchy_utils.py` is acceptable as long as `hierarchy.py` does not import `hierarchy_utils.py`, thereby avoiding a cyclic import.
tests/simulator/initialize/density_check.py (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
tests/core/numerics/interpolator/test_main.cpp (2)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
src/core/data/ions/ions.hpp (3)
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: The bulk velocity calculation in PHARE must maintain physical consistency across both ion and electron calculations. Zero density cases need special handling that preserves the physics, particularly since the calculations are used in level initialization, ion updates, and electron physics.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE's physics simulations, handling of zero density cells in velocity calculations must be based on physical requirements rather than simple numerical fixes. The solution must account for population-specific behavior in the bulk velocity computation.
Learnt from: rochSmets
PR: PHAREHUB/PHARE#921
File: src/core/data/ions/ions.hpp:138-142
Timestamp: 2025-02-06T10:52:56.461Z
Learning: In PHARE simulations, velocity calculations must account for per-population densities that can be strictly zero in given cells. Simple zero-return solutions for division by zero are inadequate as they don't reflect the correct physics.
🧬 Code Graph Analysis (4)
src/amr/level_initializer/hybrid_level_initializer.hpp (2)
src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (6)
ions(393-407)ions(393-394)ions(420-461)ions(420-421)ions(469-474)ions(469-470)src/core/numerics/ion_updater/ion_updater.hpp (4)
ions(58-59)ions(62-62)ions(73-73)ions(75-75)
tests/simulator/test_initialization.py (1)
pyphare/pyphare/pharesee/hierarchy/__init__.py (1)
hierarchy_from(13-46)
src/diagnostic/detail/types/fluid.hpp (3)
src/diagnostic/detail/h5typewriter.hpp (9)
checkCreateFileFor_(135-142)checkCreateFileFor_(135-136)diagnostic(32-32)diagnostic(34-35)diagnostic(39-41)diagnostic(54-69)diagnostic(54-54)diagnostic(91-115)diagnostic(91-95)src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (6)
ions(393-407)ions(393-394)ions(420-461)ions(420-421)ions(469-474)ions(469-470)tests/core/numerics/ion_updater/test_updater.cpp (2)
ions(325-353)ions(325-325)
tests/simulator/initialize/density_check.py (7)
pyphare/pyphare/pharesee/run/run.py (3)
Run(35-292)GetMassDensity(112-114)GetNi(116-118)tests/diagnostic/__init__.py (1)
all_timestamps(8-12)pyphare/pyphare/pharesee/hierarchy/__init__.py (1)
hierarchy_from(13-46)pyphare/pyphare/pharesee/hierarchy/fromfunc.py (4)
ions_mass_density_func1d(6-16)ions_charge_density_func1d(19-30)ions_mass_density_func2d(47-59)ions_charge_density_func2d(62-74)pyphare/pyphare/pharein/maxwellian_fluid_model.py (1)
MaxwellianFluidModel(8-303)pyphare/pyphare/pharein/electron_model.py (1)
ElectronModel(22-35)pyphare/pyphare/simulator/simulator.py (1)
reset(259-269)
🪛 Ruff (0.11.9)
tests/functional/harris/harris_2d_lb.py
75-75: Ambiguous variable name: l
(E741)
🪛 Flake8 (7.2.0)
tests/functional/harris/harris_2d_lb.py
[error] 75-75: ambiguous variable name 'l'
(E741)
🪛 Pylint (3.3.7)
tests/functional/harris/harris_2d_lb.py
[refactor] 189-189: Consider using Python 3 style super() without arguments
(R1725)
[refactor] 193-193: Consider using Python 3 style super() without arguments
(R1725)
pyphare/pyphare/pharesee/hierarchy/fromfunc.py
[refactor] 92-92: Either all return statements in a function should return an expression, or none of them should.
(R1710)
tests/simulator/initialize/density_check.py
[error] 3-3: No name 'simulator' in module 'pyphare'
(E0611)
[error] 4-4: No name 'pharesee' in module 'pyphare'
(E0611)
[error] 6-6: No name 'pharein' in module 'pyphare'
(E0611)
[error] 8-8: No name 'pharein' in module 'pyphare'
(E0611)
[error] 11-11: No name 'pharein' in module 'pyphare'
(E0611)
[error] 17-17: No name 'pharesee' in module 'pyphare'
(E0611)
[error] 18-18: No name 'pharesee' in module 'pyphare'
(E0611)
[error] 19-19: No name 'pharesee' in module 'pyphare'
(E0611)
[error] 20-20: No name 'pharesee' in module 'pyphare'
(E0611)
[error] 21-21: No name 'pharesee' in module 'pyphare'
(E0611)
[refactor] 196-196: Too many local variables (17/15)
(R0914)
[error] 216-216: Possibly using variable 'dset_h' before assignment
(E0606)
[error] 216-216: Possibly using variable 'dset_H' before assignment
(E0606)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: build (macos-13)
- GitHub Check: build (macos-latest)
- GitHub Check: build (macos-14)
- GitHub Check: build (ubuntu-latest, gcc)
- GitHub Check: build (ubuntu-latest, clang)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (cpp)
🔇 Additional comments (85)
.gitignore (1)
25-25: Ignoring.gdbinitkeeps personal debugger settings out of the repo
Standard practice; no issues spotted.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1)
39-39: Good addition for charge density support.The new mapping follows the existing pattern and enables hierarchy utilities to properly recognize and handle charge density fields alongside existing density types.
tests/functional/td/td1d.py (1)
103-103: Consistent with refactoring to explicit density types.The change from "density" to "charge_density" aligns with the broader codebase refactoring and ensures tests validate the new explicit density diagnostic types.
tests/functional/shock/shock.py (1)
110-110: Maintains consistency across functional tests.The update to "charge_density" is consistent with the refactoring effort and ensures all functional tests use the same explicit density diagnostic types.
src/phare/phare_init_small.py (1)
107-107: Correctly updates initialization script diagnostics.The change ensures initialization scripts use the new explicit charge density diagnostics, maintaining consistency between examples and the broader refactoring effort.
src/phare/phare_init.py (1)
104-104: Completes consistent refactoring across initialization scripts.The update to "charge_density" in the main initialization script ensures all diagnostic setups use explicit density types, completing the coordinated refactoring effort.
tests/functional/alfven_wave/alfven_wave1d.py (1)
99-99: LGTM: Diagnostic quantity updated to use charge densityThe change from "density" to "charge_density" aligns with the broader refactoring to explicitly handle charge density diagnostics.
tests/functional/dispersion/dispersion.py (1)
174-174: LGTM: Consistent diagnostic quantity updateThe change to "charge_density" maintains consistency with the broader refactoring across the test suite.
tests/simulator/test_run.py (1)
145-145: LGTM: Test updated for charge density diagnosticsThe change correctly updates the diagnostic quantity to "charge_density", maintaining consistency with the refactoring.
src/amr/level_initializer/hybrid_level_initializer.hpp (1)
101-101: LGTM: Method call updated for explicit charge density computationThe change from
computeDensity()tocomputeChargeDensity()improves code expressiveness by being explicit about the type of density being computed. This aligns with the broader refactoring to distinguish between particle and charge densities.src/amr/physical_models/hybrid_model.hpp (1)
157-157: LGTM: Field name updated for explicit charge density handlingThe change from
densityName()tochargeDensityName()improves code expressiveness by being explicit about the charge density field. This enhances code clarity and aligns with the systematic refactoring to distinguish between different density types.tests/core/data/ions/test_ions.cpp (1)
157-157: LGTM! Correctly updated API call for the refactoring.The change from
ions.density()(0)toions.chargeDensity()(0)correctly reflects the API refactoring to distinguish charge density from particle density.tests/simulator/refinement/test_2d_10_core.py (1)
104-104: LGTM! Correctly updated diagnostic quantity name.The change from
"density"to"charge_density"properly reflects the refactoring to explicitly distinguish charge density in fluid diagnostics.pyphare/pyphare/pharesee/plotting.py (1)
277-277: LGTM! Correctly updated data file naming convention.The change from
"ions_density.h5"to"ions_charge_density.h5"properly aligns with the refactoring to explicitly distinguish charge density data files.tests/diagnostic/test_diagnostics.ipp (1)
17-17: LGTM! Correctly updated diagnostic path.The change from
hi5.fluid("/ions/density")tohi5.fluid("/ions/charge_density")properly reflects the refactoring of diagnostic paths to explicitly use charge density terminology.tests/functional/tdtagged/td1dtagged.py (1)
123-123: LGTM! Consistent with charge density refactoring.The change from "density" to "charge_density" aligns with the systematic refactoring across the codebase to explicitly distinguish between particle density and charge density in fluid diagnostics.
tests/simulator/initialize/CMakeLists.txt (1)
23-24: LGTM! Good addition of density validation test.The new test execution for
density_check.pyappropriately validates the charge and mass density initialization, which is essential given the refactoring from generic density to specific density types.pyphare/pyphare/pharein/diagnostics.py (1)
225-225: LGTM! Core enabler for charge density diagnostics.Adding "charge_density" to the fluid_quantities list enables this diagnostic quantity throughout the codebase, supporting the systematic refactoring from generic density to explicit charge density.
tests/functional/translation/translat1d.py (2)
90-90: LGTM! Consistent diagnostic quantity update.The change to "charge_density" in the fluid diagnostics is consistent with the systematic refactoring across the codebase.
185-185: LGTM! Consistent diagnostic quantity update.The change to "charge_density" in the fluid diagnostics maintains consistency with the refactoring pattern applied across all test configurations.
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1)
148-148: LGTM! Consistent with systematic refactoring.The update to "charge_density" in the fluid diagnostics maintains consistency with the broader refactoring pattern applied across all tests and maintains proper test coverage for the new diagnostic quantities.
tests/simulator/refinement/test_2d_2_core.py (1)
104-104: LGTM! Diagnostic quantity update aligns with density refactoring.The change from
"density"to"charge_density"is consistent with the broader refactoring to distinguish between particle and charge densities throughout the codebase.tests/core/data/ion_population/test_ion_population_fixtures.hpp (1)
53-53: LGTM! Field identifier update aligns with density refactoring.The change from
"_rho"to"_particleDensity"properly reflects the new naming conventions introduced in the refactoring that distinguishes particle density from charge density.tests/diagnostic/__init__.py (2)
31-36: LGTM! Comprehensive expansion of diagnostic coverage.The addition of both
"charge_density"and"mass_density"to the top-level fluid diagnostics provides complete coverage of the new density types introduced in the refactoring.
44-44: LGTM! Population-specific charge density diagnostics added.Adding
"charge_density"alongside the existing"density"for population-specific diagnostics maintains comprehensive diagnostic coverage while supporting the new density distinctions.src/python3/patch_level.hpp (2)
40-40: LGTM! Method call updated to use charge density API.The change from
ions.density()toions.chargeDensity()correctly uses the new API introduced in the density refactoring.
62-62: LGTM! Population method call updated to charge density API.The change from
pop.density()topop.chargeDensity()properly uses the new population-specific charge density method.tests/simulator/test_initialization.py (2)
173-173: LGTM! Diagnostic quantity updated for density refactoring.The change from
"density"to"charge_density"correctly updates the test to use the new diagnostic quantity name introduced in the refactoring.
235-237: LGTM! Diagnostic file reference updated for new naming convention.The change from
"ions_density.h5"to"ions_charge_density.h5"correctly updates the filename to match the new diagnostic file naming convention introduced in the refactoring.tests/simulator/test_advance.py (2)
155-155: LGTM: Diagnostic update is consistent with the refactoring.The change from
"density"to"charge_density"correctly aligns with the broader codebase refactoring that introduces explicit charge density diagnostics.
225-225: LGTM: HDF5 filename updates are consistent.The filename changes from
"ions_density.h5"to"ions_charge_density.h5"correctly reflect the new charge density diagnostic output files.Also applies to: 234-234
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1)
436-436: LGTM: Direct ghost cell access simplifies the implementation.The change from
layout.nbrGhostFor(qty)topatch.patch_datas[qty].ghosts_nbrdirectly accesses ghost cell counts from patch data, which is consistent with the updated data handling in the charge density refactoring.src/amr/messengers/hybrid_hybrid_messenger_strategy.hpp (3)
443-447: LGTM: Interpolation correctly handles both particle and charge densities.The separation of density into
particleDensityandchargeDensityvariables and their inclusion in the interpolation call correctly implements the new physics data model while maintaining computational consistency.
454-454: LGTM: Level ghost particle interpolation properly updated.The interpolation calls for level ghost particles (both old and new) correctly pass both particle and charge densities, ensuring consistent moment computation across the AMR hierarchy.
Also applies to: 457-457
578-578: LGTM: PrepareStep correctly uses charge density.The change from
ions.density()toions.chargeDensity()in theprepareStepmethod is consistent with the refactoring and ensures the messenger uses the appropriate density field for electromagnetic calculations.src/core/numerics/ion_updater/ion_updater.hpp (2)
161-161: LGTM: Interpolator calls correctly updated for dual density fields.The interpolator calls have been systematically updated to pass both
particleDensity()andchargeDensity()parameters, which correctly reflects the separation of these physical quantities.Also applies to: 184-185, 273-274
110-110: Approve computeChargeDensity zero–density behavior
The new computeChargeDensity() implementation explicitly zeroes chargeDensity_ and then sums each population’s charge density. Any cells with zero population density remain zero in the result, matching the original behavior and preserving physical consistency. No additional special‐case handling is required here.pyphare/pyphare/pharesee/hierarchy/__init__.py (1)
14-14: LGTM: Function extension follows established patterns.The addition of the
funcparameter and corresponding logic properly extends thehierarchy_fromfunction to support functional hierarchy generation. The conditional logic is correctly positioned and the import is appropriately placed.Also applies to: 18-18, 43-44
tests/diagnostic/test_diagnostics.hpp (2)
125-125: LGTM: Test validation correctly updated for charge density.The changes from
density()tochargeDensity()are consistent with the broader refactoring and correctly validate the new charge density fields.Also applies to: 128-128
229-229: expectedPopAttrFiles correctly reflects 6 per-population file types
The loop in tests/diagnostic/test_diagnostics.hpp (lines 247–253) callsh5FileTypes.emplace_backexactly six times (domain, levelGhost, patchGhost, density, charge_density, flux), soconstexpr std::size_t expectedPopAttrFiles = 6;is accurate. No changes required.
src/core/numerics/moments/moments.hpp (2)
18-20: LGTM: Moment reset correctly handles dual density fields.The separation of the single
density().zero()operation into separateparticleDensity().zero()andchargeDensity().zero()operations is correct and ensures both fields are properly initialized.
44-46: LGTM: Interpolation calls systematically updated for dual densities.The addition of separate
particleDensityandchargeDensityvariables and their consistent use in all interpolation calls correctly implements the dual density moment computation.Also applies to: 51-51, 56-56, 61-61
src/diagnostic/detail/types/fluid.hpp (3)
149-150: LGTM: File creation logic correctly extended for charge density.The addition of "charge_density" to the diagnostic file creation checks for both populations and ions is systematic and consistent with the broader refactoring.
Also applies to: 154-155
202-204: Note the semantic change in density handling.For populations, "density" now refers to
particleDensity()while "charge_density" refers tochargeDensity(). For ions, onlychargeDensity()is used. This semantic shift is consistent but worth noting for future reference.Also applies to: 212-213
274-275: LGTM: Dataset operations consistently handle charge density.The initialization, writing, and attribute handling for charge density datasets are systematically implemented across all relevant methods, maintaining consistency with the existing patterns.
Also applies to: 283-284, 315-317, 325-326, 355-355
tests/core/numerics/interpolator/test_main.cpp (6)
517-517: LGTM: Charge density grid addition is correct.The addition of the
rho_cgrid for charge density alongside the existingrhogrid properly supports the dual-density interpolation model. The grid initialization and usage are consistent.Also applies to: 533-533
523-527: Clear documentation of test setup.The updated comment effectively explains the test configuration where particles are arranged to produce particle density of 1 and charge density of 2, making the test expectations clear.
652-652: Correct interpolator call signature update.The interpolator call properly includes both
rhoandrho_cparameters, correctly implementing the new dual-density interface.
670-670: Proper test assertion for charge density.The test correctly validates that
rho_cat the target index equals 2.0, ensuring the charge density deposition works as expected.
699-699: Consistent 2D test structure.The 2D test fixture correctly includes the
rho_cgrid and passes it to the interpolator, maintaining consistency with the 1D test pattern.Also applies to: 706-706, 720-720
541-560: Charge values yield the correct normalized density of 2.0All three interpolation orders produce a weighted‐average charge of 2.0 once you divide the raw deposition by the sum of the weights at the target cell:
Order 1:
• Raw sum = 1.0⋅2.0 + 0.4⋅1.85 + 0.6⋅2.1 = 4.0
• Total weight = 1.0 + 0.4 + 0.6 = 2.0
• 4.0 / 2.0 = 2.0Order 2:
• Raw sum = 1.0⋅2.0 + 0.2⋅3.2 + 0.8⋅1.7 + 1.0⋅2.0 = 6.0
• Total weight = 1.0 + 0.2 + 0.8 + 1.0 = 3.0
• 6.0 / 3.0 = 2.0Order 3:
• Raw sum = 1.0⋅2.0 + 1.0⋅2.0 + 1.0⋅2.0 + 0.1⋅3.35 + 0.9⋅1.85 = 8.0
• Total weight = 1.0 + 1.0 + 1.0 + 0.1 + 0.9 = 4.0
• 8.0 / 4.0 = 2.0No changes are required—the test’s expected 2.0 charge density is correct.
pyphare/pyphare/pharesee/run/run.py (3)
30-30: Consistent file naming convention update.The change from
"ions_density"to"ions_charge_density"in the quantities mapping correctly reflects the new diagnostic file naming convention.
117-117: GetNi method correctly updated for charge density.The method now loads ion charge density from the appropriately named file, maintaining the semantic meaning of "Ni" as ion density in the physics context.
154-154: Electron pressure calculation uses correct density.The GetPe method correctly uses charge density for electron pressure calculation, which is physically appropriate since electron pressure depends on charge-weighted ion density.
src/core/numerics/interpolator/interpolator.hpp (3)
472-475: Clear parameter naming for dual-density interface.The parameter names
particleDensityandchargeDensityclearly distinguish the two density types, improving code readability and reducing potential confusion.
490-496: Correct physics implementation for dual deposition.The deposition logic properly implements the physics:
- Particle density: deposits constant weight (1.0) representing particle count
- Charge density: deposits particle charge representing charge contribution
This correctly separates particle counting from charge-weighted quantities.
509-513: Consistent forwarding operator signature.The rvalue reference forwarding operator correctly maintains the same dual-density interface, ensuring consistent API usage for both lvalue and rvalue particle ranges.
src/core/data/electrons/electrons.hpp (3)
68-68: Correct density accessor update for electron models.The density() methods now correctly return ion charge density instead of particle density. This is physically appropriate since electron models require charge-weighted ion density for quasi-neutrality and current calculations.
Also applies to: 81-81
115-115: Physics-correct electron velocity calculation.The bulk velocity computation correctly uses ion charge density (
ions_.chargeDensity()) in the electron velocity formula:Ve = Vi - J/Ne. This maintains physical consistency since current density J is charge-based and requires charge density for proper normalization.Also applies to: 122-122, 129-131
214-214: Appropriate electron pressure calculation.The isothermal electron pressure closure correctly uses ion charge density (
ions_.chargeDensity()) for computingPe = Ne * Te. This maintains quasi-neutrality assumptions where electron density equals ion charge density.tests/functional/harris/harris_2d_lb.py (2)
137-138: Correct usage of charge_density diagnostics.The fluid diagnostics correctly use
"charge_density"instead of the deprecated generic"density", properly exercising the updated diagnostic infrastructure introduced in this PR.Also applies to: 140-142
40-148: Well-structured simulation configuration.The simulation setup comprehensively covers all aspects needed for a Harris current sheet test: appropriate domain size, AMR configuration, initial conditions with hyperbolic tangent profiles, and comprehensive diagnostics. The optional load balancing integration is well-implemented.
src/core/data/ions/ion_population/ion_population.hpp (4)
29-39: Constructor initialization looks good.The initialization of
particleDensity_andchargeDensity_fields follows the established naming convention and properly uses the HybridQuantity::Scalar::rho centering.
48-56: Resource state checks properly updated.Both
isUsable()andisSettable()correctly include the newparticleDensity_andchargeDensity_fields in their validation logic.
79-83: Accessor methods properly implement const-correctness.The split into separate
particleDensity()andchargeDensity()accessors with both const and non-const variants follows best practices.
100-104: Resource management properly updated.The
getCompileTimeResourcesViewList()correctly includes both density fields in the resource tuple, maintaining consistency with the ResourcesUser interface pattern.Also applies to: 127-128
pyphare/pyphare/pharesee/hierarchy/fromfunc.py (3)
6-30: 1D density computation functions are well-implemented.The functions correctly compute mass-weighted and charge-weighted densities by summing contributions from multiple ion populations. Good use of numpy for efficiency.
47-74: 2D density functions correctly handle coordinate meshgrids.The functions properly use
np.meshgrid(y, x)to create coordinate arrays and sum across the population dimension. The implementation is consistent with the 1D versions.
33-89: Hierarchy wrapper functions properly integrate with the framework.The
hierarchy_from_func1dandhierarchy_from_func2dfunctions correctly extract coordinates from patch data and return the expected data structure forcompute_hier_from.tests/core/numerics/ion_updater/test_updater.cpp (3)
214-219: Test buffer struct properly updated for dual density model.The addition of separate
particleDensityandchargeDensityfields for each population correctly reflects the refactored ion data model.
241-254: Constructor and buffer management correctly updated.The constructors properly initialize all density fields with descriptive names, and
setBuffers()correctly assigns them using structured bindings. The updates maintain consistency with the refactored ion data structures.Also applies to: 278-291, 325-353
543-543: Test methods comprehensively updated for new density model.All test methods correctly use the split density fields. The interpolation calls properly pass both
particleDensityandchargeDensity, and density validation appropriately checksparticleDensityagainst the prescribed functions.Also applies to: 556-567, 577-588, 615-626, 669-673
tests/core/data/electrons/test_electrons.cpp (2)
151-153: Test fixture members properly expanded for new density model.The addition of separate particle and charge density fields for ions and protons, along with distinct tensor fields, correctly supports testing of the refactored electron model.
Also applies to: 166-167, 196-205
164-187: Test logic correctly validates electron-ion charge density equality.The test properly uses
ions.chargeDensity()to validate that electron density equals ion charge density, maintaining the physical correctness of the charge neutrality condition.Also applies to: 247-247, 276-277, 323-324, 373-373
tests/simulator/initialize/density_check.py (3)
1-32: Test setup and imports look appropriate.The global parameters and matplotlib backend configuration are suitable for automated testing. The pylint import errors appear to be false positives related to the PHARE module structure.
58-111: Simulation configurations are comprehensive and well-structured.Both 1D and 2D configurations properly set up multi-population simulations with appropriate diagnostics for validating the new density model. Good coverage of both fluid-level and population-level quantities.
Also applies to: 140-193
198-201: Good practice resetting simulator state between tests.Properly clearing the global simulation state with
reset()and settingph.global_vars.sim = Noneensures clean test isolation.src/core/data/ions/ions.hpp (8)
44-45: Constructor initialization looks good.The dual initialization of
massDensity_andchargeDensity_properly reflects the new density model.
60-65: Density accessor methods are well-implemented.The const and non-const overloads for both density types maintain proper const-correctness.
75-91: Charge density computation is correctly implemented.The accumulation pattern using
std::transformwithstd::plusis efficient and clear.
102-108: Mass density calculation correctly weights particle densities.The lambda properly accumulates mass-weighted particle densities from each population.
175-176: Resource usability checks properly updated.Both density fields are correctly included in the usability validation.
189-190: Settability validation correctly includes both densities.The method properly ensures all density fields are settable.
214-215: Resource view list properly updated.Both density fields are correctly exposed for resource management.
238-240: Member variables correctly declared.The charge and mass density fields are properly defined with consistent types.
| def hierarchy_from_func(func, hier, **kwargs): | ||
| if hier.ndim == 1: | ||
| return hierarchy_from_func1d(func, hier, **kwargs) | ||
| if hier.ndim == 2: | ||
| return hierarchy_from_func2d(func, hier, **kwargs) | ||
|
|
There was a problem hiding this comment.
Add error handling for unsupported dimensions.
The function implicitly returns None for dimensions other than 1 or 2. This should explicitly raise an error to provide clear feedback.
Add explicit error handling:
def hierarchy_from_func(func, hier, **kwargs):
if hier.ndim == 1:
return hierarchy_from_func1d(func, hier, **kwargs)
if hier.ndim == 2:
return hierarchy_from_func2d(func, hier, **kwargs)
+ raise ValueError(f"Unsupported dimension: {hier.ndim}. Only 1D and 2D are supported.")📝 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 hierarchy_from_func(func, hier, **kwargs): | |
| if hier.ndim == 1: | |
| return hierarchy_from_func1d(func, hier, **kwargs) | |
| if hier.ndim == 2: | |
| return hierarchy_from_func2d(func, hier, **kwargs) | |
| def hierarchy_from_func(func, hier, **kwargs): | |
| if hier.ndim == 1: | |
| return hierarchy_from_func1d(func, hier, **kwargs) | |
| if hier.ndim == 2: | |
| return hierarchy_from_func2d(func, hier, **kwargs) | |
| + raise ValueError(f"Unsupported dimension: {hier.ndim}. Only 1D and 2D are supported.") |
🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 92-92: Either all return statements in a function should return an expression, or none of them should.
(R1710)
🤖 Prompt for AI Agents
In pyphare/pyphare/pharesee/hierarchy/fromfunc.py around lines 92 to 97, the
function hierarchy_from_func currently returns None implicitly for hier.ndim
values other than 1 or 2. Add explicit error handling by raising a ValueError
with a clear message indicating that only 1D and 2D hierarchies are supported.
This will provide clear feedback when unsupported dimensions are passed.
| def assert_close_enough(h, H): | ||
| for lvl_h, lvl_H in zip(h.levels(time).values(), H.levels(time).values()): | ||
| for patch_h, patch_H in zip(lvl_h.patches, lvl_H.patches): | ||
| pd_h = patch_h.patch_datas["value"] | ||
| pd_H = patch_H.patch_datas["value"] | ||
| ghosts_num = pd_h.ghosts_nbr[0] | ||
|
|
||
| if pd_H.ndim == 1: | ||
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num] | ||
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num] | ||
| if pd_H.ndim == 2: | ||
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | ||
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | ||
|
|
||
| std = np.std(dset_h - dset_H) | ||
| print("dim = {}, sigma(user v - actual v) = {}".format(pd_H.ndim, std)) | ||
| assert( std < 0.06 ) # empirical value obtained from print just above |
There was a problem hiding this comment.
Fix potential undefined variables and use proper assertions.
The function has undefined variable issues if pd_H.ndim is neither 1 nor 2. Also, use proper test assertions instead of bare assert.
Fix the conditional logic and assertion:
def assert_close_enough(h, H):
for lvl_h, lvl_H in zip(h.levels(time).values(), H.levels(time).values()):
for patch_h, patch_H in zip(lvl_h.patches, lvl_H.patches):
pd_h = patch_h.patch_datas["value"]
pd_H = patch_H.patch_datas["value"]
ghosts_num = pd_h.ghosts_nbr[0]
if pd_H.ndim == 1:
dset_h = pd_h.dataset[ghosts_num:-ghosts_num]
dset_H = pd_H.dataset[ghosts_num:-ghosts_num]
- if pd_H.ndim == 2:
+ elif pd_H.ndim == 2:
dset_h = pd_h.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num]
dset_H = pd_H.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num]
+ else:
+ raise ValueError(f"Unsupported patch data dimension: {pd_H.ndim}")
std = np.std(dset_h - dset_H)
print("dim = {}, sigma(user v - actual v) = {}".format(pd_H.ndim, std))
- assert( std < 0.06 ) # empirical value obtained from print just above
+ np.testing.assert_less(std, 0.06, f"Standard deviation {std} exceeds threshold")📝 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 assert_close_enough(h, H): | |
| for lvl_h, lvl_H in zip(h.levels(time).values(), H.levels(time).values()): | |
| for patch_h, patch_H in zip(lvl_h.patches, lvl_H.patches): | |
| pd_h = patch_h.patch_datas["value"] | |
| pd_H = patch_H.patch_datas["value"] | |
| ghosts_num = pd_h.ghosts_nbr[0] | |
| if pd_H.ndim == 1: | |
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num] | |
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num] | |
| if pd_H.ndim == 2: | |
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | |
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | |
| std = np.std(dset_h - dset_H) | |
| print("dim = {}, sigma(user v - actual v) = {}".format(pd_H.ndim, std)) | |
| assert( std < 0.06 ) # empirical value obtained from print just above | |
| def assert_close_enough(h, H): | |
| for lvl_h, lvl_H in zip(h.levels(time).values(), H.levels(time).values()): | |
| for patch_h, patch_H in zip(lvl_h.patches, lvl_H.patches): | |
| pd_h = patch_h.patch_datas["value"] | |
| pd_H = patch_H.patch_datas["value"] | |
| ghosts_num = pd_h.ghosts_nbr[0] | |
| if pd_H.ndim == 1: | |
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num] | |
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num] | |
| elif pd_H.ndim == 2: | |
| dset_h = pd_h.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | |
| dset_H = pd_H.dataset[ghosts_num:-ghosts_num, ghosts_num:-ghosts_num] | |
| else: | |
| raise ValueError(f"Unsupported patch data dimension: {pd_H.ndim}") | |
| std = np.std(dset_h - dset_H) | |
| print("dim = {}, sigma(user v - actual v) = {}".format(pd_H.ndim, std)) | |
| np.testing.assert_less(std, 0.06, f"Standard deviation {std} exceeds threshold") |
🧰 Tools
🪛 Pylint (3.3.7)
[error] 216-216: Possibly using variable 'dset_h' before assignment
(E0606)
[error] 216-216: Possibly using variable 'dset_H' before assignment
(E0606)
🤖 Prompt for AI Agents
In tests/simulator/initialize/density_check.py around lines 202 to 218, the
function assert_close_enough does not handle cases where pd_H.ndim is not 1 or
2, leading to potential undefined variables dset_h and dset_H. Also, it uses
bare assert statements which should be replaced with proper test framework
assertions. Fix this by adding an else clause to handle unexpected ndim values,
raising an error or skipping, and replace bare assert with a proper assertion
method like self.assertTrue or pytest's assert with a clear message.
| auto& Bz = model.state.electromag.B.getComponent(PHARE::core::Component::Z); | ||
|
|
||
| auto& N = model.state.ions.density(); | ||
| auto& N = model.state.ions.chargeDensity(); |
Check notice
Code scanning / CodeQL
Unused local variable Note
| auto static _ions(Args&... args) | ||
| { | ||
| auto const& [Fi, Nibuffer, NiProtons, Vi, M, protons_M, pack] | ||
| auto const& [ionFlux, ionChargeDensity, ionMassDensity, protonParticleDensity, |
Check notice
Code scanning / CodeQL
Unused local variable Note test
| { | ||
| auto const& [V, m, d, md] = ions.getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&Nibuffer); | ||
| auto const& [V, m, d_c, d_m] = ions.getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable Note test
| { | ||
| { | ||
| auto const& [V, m, d, md] = ions.getCompileTimeResourcesViewList(); | ||
| auto const& [V, m, cd, md] = ions.getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable Note test
| { | ||
| auto const& [F, M, d, particles] = pops[0].getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&protonDensity); | ||
| auto const& [F, M, d, c, particles] = pops[0].getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable Note test
| { | ||
| auto const& [F, M, d, particles] = pops[1].getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&alphaDensity); | ||
| auto const& [F, M, d, c, particles] = pops[1].getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable Note test
| auto& Bz = model.state.electromag.B.getComponent(PHARE::core::Component::Z); | ||
|
|
||
| auto& N = model.state.ions.density(); | ||
| auto& N = model.state.ions.chargeDensity(); |
Check notice
Code scanning / CodeQL
Unused local variable
| auto static _ions(Args&... args) | ||
| { | ||
| auto const& [Fi, Nibuffer, NiProtons, Vi, M, protons_M, pack] | ||
| auto const& [ionFlux, ionChargeDensity, ionMassDensity, protonParticleDensity, |
Check notice
Code scanning / CodeQL
Unused local variable
| { | ||
| auto const& [V, m, d, md] = ions.getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&Nibuffer); | ||
| auto const& [V, m, d_c, d_m] = ions.getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable
| d.setBuffer(&NiProtons); | ||
| Fi.set_on(F); | ||
| protons_M.set_on(m); | ||
| auto const& [F, m, Np, Nc, poppack] = pops[0].getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable
| { | ||
| { | ||
| auto const& [V, m, d, md] = ions.getCompileTimeResourcesViewList(); | ||
| auto const& [V, m, cd, md] = ions.getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable
| { | ||
| auto const& [F, M, d, particles] = pops[0].getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&protonDensity); | ||
| auto const& [F, M, d, c, particles] = pops[0].getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable
| { | ||
| auto const& [F, M, d, particles] = pops[1].getCompileTimeResourcesViewList(); | ||
| d.setBuffer(&alphaDensity); | ||
| auto const& [F, M, d, c, particles] = pops[1].getCompileTimeResourcesViewList(); |
Check notice
Code scanning / CodeQL
Unused local variable
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
Co-authored-by: roch smets <[email protected]> Co-authored-by: Nicolas Aunai <[email protected]>
3 participants
replaces #921
rebased to master
plotting stuff removed