Merged
Conversation
📝 Walkthrough## Walkthrough
The changes introduce a new fluid diagnostic for the "pressure_tensor" quantity associated with the "protons" population in the simulation configuration for both Harris 2D and simulator test runs. Plotting functions are updated to visualize the "Pressure" quantity at each timestamp, saving the results with descriptive filenames and enabling patch plotting. Existing diagnostics and parameters remain unchanged.
## Changes
| File(s) | Change Summary |
|-------------------------------------------------------------|---------------------------------------------------------------------------------------------------------------------|
| tests/functional/harris/harris_2d.py, tests/simulator/test_run.py | Added "pressure_tensor" diagnostic for the "protons" population; extended plotting to include "Pressure" visualization at each timestamp with patch plotting and timestamped filenames. |
| pyphare/pyphare/pharesee/run/utils.py | Modified `_compute_pop_pressure` to source flux and density arrays from fixed keys ("x", "y", "z", "value") in patch data instead of population-specific keys; calculation logic unchanged. |
| src/diagnostic/detail/h5typewriter.hpp | Added protected method `h5FileForQuantity` to retrieve HDF5 file reference for a diagnostic quantity, throwing if unknown. |
| src/diagnostic/detail/types/{electromag,fluid,info,meta,particle}.hpp | Replaced direct `fileData_` map access with calls to `Super::h5FileForQuantity(diagnostic)` for obtaining HDF5 file references in diagnostic writer classes; no logic or control flow changes. |
## Possibly related PRs
- PHAREHUB/PHARE#1027: Extends Harris test diagnostics and plotting, providing a foundation for the additional pressure tensor and pressure diagnostics introduced in this PR.
## Suggested reviewers
- nicolasaunai📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (9)
⏰ 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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/functional/harris/harris_2d.py(2 hunks)tests/simulator/test_run.py(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
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: 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: 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.
tests/simulator/test_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: 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/harris/harris_2d.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.
🪛 Ruff (0.11.9)
tests/simulator/test_run.py
200-200: Undefined name ru
(F821)
tests/functional/harris/harris_2d.py
146-146: Undefined name pop
(F821)
187-187: Undefined name ru
(F821)
⏰ 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). (6)
- GitHub Check: build (macos-14)
- GitHub Check: build (macos-13)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (python)
- GitHub Check: build (ubuntu-latest, clang)
- GitHub Check: build (ubuntu-latest, gcc)
PhilipDeegan
force-pushed
the
pressureplot
branch
5 times, most recently
from
3910b4b to
8cb128a
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
pyphare/pyphare/pharesee/run/utils.py (1)
420-422: Consider adding validation for required keys.The function now depends on specific hardcoded keys ("x", "y", "z") being present in
patch_datas. Adding validation would make the code more robust.+# Validate required keys exist +required_keys = ["x", "y", "z", "value"] +missing_keys = [key for key in required_keys if key not in patch_datas] +if missing_keys: + raise KeyError(f"Missing required keys in patch_datas: {missing_keys}") + Fx = patch_datas["x"].dataset[:] Fy = patch_datas["y"].dataset[:] Fz = patch_datas["z"].dataset[:]
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyphare/pyphare/pharesee/run/utils.py(1 hunks)tests/functional/harris/harris_2d.py(4 hunks)tests/simulator/test_run.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/simulator/test_run.py
- tests/functional/harris/harris_2d.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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: 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: 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.
pyphare/pyphare/pharesee/run/utils.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: 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.
⏰ 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)
- GitHub Check: Analyze (cpp)
- GitHub Check: Analyze (python)
- GitHub Check: build (macos-14)
- GitHub Check: build (macos-13)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest, gcc)
- GitHub Check: build (ubuntu-latest, clang)
PhilipDeegan
force-pushed
the
pressureplot
branch
from
8cb128a to
ae45280
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
pyphare/pyphare/pharesee/run/utils.py(2 hunks)tests/functional/harris/harris_2d.py(4 hunks)tests/simulator/test_run.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/functional/harris/harris_2d.py
- pyphare/pyphare/pharesee/run/utils.py
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
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.
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: 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: 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.
tests/simulator/test_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: 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.
🪛 Ruff (0.11.9)
tests/simulator/test_run.py
158-158: Loop control variable quantity not used within loop body
(B007)
⏰ 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)
- GitHub Check: build (macos-14)
- GitHub Check: build (macos-latest)
- GitHub Check: build (macos-13)
- GitHub Check: build (ubuntu-latest, clang)
- GitHub Check: build (ubuntu-latest, gcc)
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (cpp)
🔇 Additional comments (3)
tests/simulator/test_run.py (3)
145-150: LGTM! Pressure tensor diagnostic addition looks correct.The addition of "pressure_tensor" to the fluid diagnostics aligns with the PR objectives and enhances the simulation's diagnostic capabilities.
171-171: Good practice: Centralized population name management.The introduction of the
pop_namevariable improves code maintainability by centralizing the population name definition.
199-218: Plotting implementation looks comprehensive and well-structured.The pressure tensor plotting calls correctly handle both population-specific components (Pxx, Pzz for protons) and global pressure components (Pi). The use of the
pop_namevariable ensures consistency across the plotting calls.
PhilipDeegan
force-pushed
the
pressureplot
branch
2 times, most recently
from
d21070d to
195cba7
Compare
PhilipDeegan
force-pushed
the
pressureplot
branch
from
195cba7 to
94978e9
Compare
nicolasaunai
approved these changes
Jul 11, 2025
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.