Conversation
WalkthroughWalkthroughThe recent updates to the Changes
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 43
Outside diff range, codebase verification and nitpick comments (4)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (2)
117-117: Rename unused loop variable.The loop control variable
lvlis not used within the loop body. Rename it to_lvlto indicate it is intentionally unused.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Rename unused loop variable.The loop control variable
lvlis not used within the loop body. Rename it to_lvlto indicate it is intentionally unused.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1)
453-551: Cyclic Import DetectedThere is a cyclic dependency between
pyphare/pyphare/pharesee/hierarchy/hierarchy.pyandpyphare/pyphare/pharesee/plotting.py.
hierarchy.pyimportsdist_plotfromplotting.plotting.pyindirectly imports fromhierarchy.This cyclic dependency needs to be resolved to ensure proper functionality and maintainability of the code.
Analysis chain
LGTM! Ensure necessary imports are present.
The code changes are approved.
However, verify that all necessary imports are present and there are no cyclic imports.
Let's correct the approach to verify the necessary imports and check for cyclic dependencies.
To ensure there are no cyclic imports, we need to check if
pyphare/pyphare/pharesee/plotting.pyimportspyphare.pharesee.hierarchy.hierarchy.
To ensure there are no indirect cyclic imports, we need to check the imports in
pyphare/pyphare/pharesee/hierarchy/fromh5.py.
To confirm the specific imports in
hierarchy.pyand ensure no cyclic dependencies are present, we need to check the imports inpyphare/pyphare/pharesee/hierarchy/hierarchy.py.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all necessary imports are present and there are no cyclic imports. # Test: Check for cyclic imports. Expect: No cyclic imports. rg --type python 'import' | grep pyphare.pharesee.plottingLength of output: 89
Script:
#!/bin/bash # Description: Verify that all necessary imports are present and there are no cyclic imports. # Find all Python files and search for import statements within them. fd -e py -x rg 'import' {} | grep pyphare.pharesee.plottingLength of output: 115
Script:
#!/bin/bash # Description: Check for cyclic imports between plotting and hierarchy modules. # Search for imports in pyphare/pyphare/pharesee/plotting.py fd -e py -g 'plotting.py' -x rg 'import' {}Length of output: 886
Script:
#!/bin/bash # Description: Check for indirect cyclic imports in pyphare/pyphare/pharesee/hierarchy/fromh5.py. # Search for imports in pyphare/pyphare/pharesee/hierarchy/fromh5.py fd -e py -g 'fromh5.py' -x rg 'import' {}Length of output: 524
Script:
#!/bin/bash # Description: Check imports in pyphare/pyphare/pharesee/hierarchy/hierarchy.py to confirm no cyclic dependencies. # Search for imports in pyphare/pyphare/pharesee/hierarchy/hierarchy.py fd -e py -g 'hierarchy.py' -x rg 'import' {}Length of output: 835
Tools
Ruff
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
tests/simulator/test_advance.py (1)
362-362: Rename unused loop variable.The loop control variable
lvlis not used within the loop body. Rename it to_lvlto indicate it is intentionally unused.- for ilvl, lvl in datahier.levels().items(): + for ilvl, _lvl in datahier.levels().items():Tools
Ruff
362-362: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (47)
- pyphare/pyphare/core/box.py (1 hunks)
- pyphare/pyphare/core/operators.py (1 hunks)
- pyphare/pyphare/core/phare_utilities.py (1 hunks)
- pyphare/pyphare/cpp/init.py (1 hunks)
- pyphare/pyphare/pharein/init.py (2 hunks)
- pyphare/pyphare/pharein/diagnostics.py (2 hunks)
- pyphare/pyphare/pharein/examples/job.py (8 hunks)
- pyphare/pyphare/pharein/maxwellian_fluid_model.py (1 hunks)
- pyphare/pyphare/pharein/restarts.py (2 hunks)
- pyphare/pyphare/pharein/simulation.py (5 hunks)
- pyphare/pyphare/pharesee/geometry.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/init.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromh5.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromsim.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patch.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchlevel.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/scalarfield.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/vectorfield.py (1 hunks)
- pyphare/pyphare/pharesee/particles.py (1 hunks)
- pyphare/pyphare/pharesee/plotting.py (1 hunks)
- pyphare/pyphare/pharesee/run/init.py (1 hunks)
- pyphare/pyphare/pharesee/run/run.py (1 hunks)
- pyphare/pyphare/pharesee/run/utils.py (4 hunks)
- pyphare/pyphare/simulator/simulator.py (1 hunks)
- pyphare/pyphare_tests/test_pharesee/CMakeLists.txt (1 hunks)
- pyphare/pyphare_tests/test_pharesee/init.py (3 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry.py (2 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (6 hunks)
- pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1 hunks)
- tests/amr/data/field/coarsening/test_coarsen_field.py (1 hunks)
- tests/amr/data/field/refine/test_refine_field.py (1 hunks)
- tests/core/data/gridlayout/test_laplacian.py (1 hunks)
- tests/functional/alfven_wave/alfven_wave1d.py (1 hunks)
- tests/functional/conservation/conserv.py (3 hunks)
- tests/functional/tdtagged/td1dtagged.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 (6 hunks)
- tests/simulator/test_diagnostic_timestamps.py (2 hunks)
- tests/simulator/test_diagnostics.py (1 hunks)
- tests/simulator/test_initialization.py (2 hunks)
- tests/simulator/test_restarts.py (4 hunks)
- tests/simulator/test_run.py (6 hunks)
- tools/python3/phloping.py (1 hunks)
Files skipped from review due to trivial changes (10)
- pyphare/pyphare/core/phare_utilities.py
- pyphare/pyphare/cpp/init.py
- pyphare/pyphare/pharein/init.py
- pyphare/pyphare/pharein/diagnostics.py
- pyphare/pyphare/pharein/maxwellian_fluid_model.py
- pyphare/pyphare/pharein/restarts.py
- pyphare/pyphare/pharesee/particles.py
- pyphare/pyphare/pharesee/run/init.py
- pyphare/pyphare/simulator/simulator.py
- tests/simulator/test_diagnostic_timestamps.py
Additional context used
Learnings (1)
tests/simulator/test_restarts.py (1)
Learnt from: PhilipDeegan PR: PHAREHUB/PHARE#784 File: tests/simulator/test_restarts.py:333-339 Timestamp: 2023-11-25T19:21:40.002Z Learning: The user has indicated that minimal changes were made to get the function working, but acknowledges that there may still be redundant code present, such as sleep/wait/advances. The user is open to further streamlining the code.
GitHub Check: CodeQL
pyphare/pyphare/pharesee/hierarchy/patch.py
[notice] 49-49: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/vectorfield.py
[notice] 85-85: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[notice] 57-57: Unused local variable
Variable self is not used.
[notice] 58-58: Unused local variable
Variable other is not used.
[notice] 80-80: Unused local variable
Variable self is not used.
[notice] 81-81: Unused local variable
Variable other is not used.pyphare/pyphare/pharesee/hierarchy/patchdata.py
[warning] 123-123: Overwriting attribute in super-class or sub-class
Assignment overwrites attribute layout, which was previously defined in superclass PatchData.pyphare/pyphare/pharesee/plotting.py
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.pyphare/pyphare/pharesee/hierarchy/fromh5.py
[notice] 8-8: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 9-9: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 17-17: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy_utils begins an import cycle.pyphare/pyphare/pharesee/run/run.py
[notice] 261-261: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
[notice] 38-38: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 236-236: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 12-12: Unused import
Import of '_current1d' is not used.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
[notice] 187-187: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 193-193: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.
[notice] 230-230: Imprecise assert
assertTrue(a < b) cannot provide an informative message. Using assertLess(a, b) instead will give more informative messages.pyphare/pyphare_tests/test_pharesee/test_geometry.py
[notice] 295-295: Imprecise assert
assertTrue(a is b) cannot provide an informative message. Using assertIs(a, b) instead will give more informative messages.pyphare/pyphare/pharesee/run/utils.py
[notice] 349-349: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
[notice] 1-1: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.pyphare/pyphare/pharesee/hierarchy/hierarchy.py
[notice] 553-553: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[warning] 143-143: Comparison of identical values
Comparison of identical values; use cmath.isnan() if testing for not-a-number.
[failure] 186-186: Potentially uninitialized local variable
Local variable 'slice_dim' may be used before it is initialized.
[failure] 187-187: Potentially uninitialized local variable
Local variable 'cst_dim' may be used before it is initialized.
[notice] 565-565: Cyclic import
Import of module pyphare.pharesee.plotting begins an import cycle.
[notice] 563-563: Module is imported with 'import' and 'import from'
Module 'copy' is imported with both 'import' and 'import from'.
Ruff
tools/python3/phloping.py
53-53: SyntaxError: Unexpected indentation
pyphare/pyphare/pharesee/hierarchy/fromh5.py
27-27: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
98-98: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
246-249: Use ternary operator
selection_box = hier.selection_box[time] if hier.selection_box is not None else Noneinstead ofif-else-blockReplace
if-else-block withselection_box = hier.selection_box[time] if hier.selection_box is not None else None(SIM108)
320-320: Do not
assert False(python -Oremoves these calls), raiseAssertionError()Replace
assert False(B011)
pyphare/pyphare/pharesee/run/run.py
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
54-54: Ambiguous variable name:
l(E741)
tests/simulator/test_restarts.py
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
pyphare/pyphare/pharesee/run/utils.py
264-269: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
91-91: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
26-26: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
245-245: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
246-246: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
270-270: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
593-593: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
626-626: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
638-638: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
tests/simulator/test_advance.py
362-362: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
Additional comments not posted (123)
pyphare/pyphare_tests/test_pharesee/CMakeLists.txt (1)
8-8: Addition of new test for hierarchyThe line correctly adds a new test for the hierarchy module. Ensure that the test file
test_hierarchy.pyexists and is correctly implemented.pyphare/pyphare/pharesee/hierarchy/patchlevel.py (2)
1-15: Class definition and initializationThe
PatchLevelclass initialization seems correct. Ensure thatpatchesis always a list of valid patch objects.
Line range hint
16-29:
Methods for iteration and range calculationThe methods
__iter__andlevel_rangeare well-defined. Ensure that patches always contain validpatch_dataswithxattributes.pyphare/pyphare/pharesee/hierarchy/__init__.py (3)
1-4: Updated importsThe imports are correctly updated to reflect the refactoring. Ensure that the imported modules and functions exist and are correctly implemented.
6-10: Updated__all__listThe
__all__list is correctly updated to include the new imports.
13-42: Definition ofhierarchy_fromfunctionThe function definition is correct. Ensure that
hierarchy_fromh5andhierarchy_from_simare correctly implemented and handle the provided arguments.pyphare/pyphare/pharesee/hierarchy/patch.py (3)
1-31: Class definition and initializationThe
Patchclass initialization seems correct. Ensure thatpatch_datasis always a list of validPatchDataobjects.
32-38: String representation methodsThe
__str__and__repr__methods are correctly implemented.
39-47: Copy methodsThe
copyand__copy__methods are correctly implemented. Ensure that thePatchDataclass supports deep copying.pyphare/pyphare/pharesee/hierarchy/vectorfield.py (3)
15-29: LGTM!The constructor correctly initializes the
VectorFieldby renaming the hierarchy and calling the superclass constructor.
31-34: LGTM!The multiplication operation is correctly implemented with proper type checking and returning a new
VectorField.
36-37: LGTM!The right multiplication is correctly delegated to the
__mul__method.pyphare/pyphare/pharein/examples/job.py (9)
Line range hint
11-22:
LGTM!The
Simulationconfiguration block correctly sets various parameters for the simulation.
43-45: LGTM!The function
bxcorrectly defines the magnetic field as a cosine function.
47-47: LGTM!The
MaxwellianFluidModelconfiguration block correctly sets the magnetic field and proton density.
50-50: LGTM!The
ElectronModelconfiguration block correctly sets the closure and temperature.
Line range hint
53-61:
LGTM!The first
ElectromagDiagnosticsconfiguration block correctly sets the diagnostics for the electric field.
Line range hint
63-72:
LGTM!The first
FluidDiagnosticsconfiguration block correctly sets the diagnostics for the proton density.
Line range hint
74-81:
LGTM!The second
FluidDiagnosticsconfiguration block correctly sets the diagnostics for the bulk velocity of the background population.
Line range hint
83-90:
LGTM!The third
FluidDiagnosticsconfiguration block correctly sets the diagnostics for the density of all populations.
Line range hint
92-99:
LGTM!The fourth
FluidDiagnosticsconfiguration block correctly sets the diagnostics for the flux of the background population.pyphare/pyphare/core/operators.py (9)
8-19: LGTM!The function
_compute_dot_productcorrectly computes the dot product of two datasets and returns the expected structure.
22-29: LGTM!The function
_compute_sqrtcorrectly computes the square root of a dataset and returns the expected structure.
32-52: LGTM!The function
_compute_cross_productcorrectly computes the cross product of two datasets and returns the expected structure.
55-80: LGTM!The function
_compute_gradcorrectly computes the gradient of a dataset and returns the expected structure.
83-99: LGTM!The function
dotcorrectly computes the dot product of two hierarchical data structures and returns aScalarField.
102-118: LGTM!The function
crosscorrectly computes the cross product of two hierarchical data structures and returns aVectorField.
121-127: LGTM!The function
sqrtcorrectly computes the square root of a hierarchical data structure and returns aScalarField.
130-133: LGTM!The function
moduluscorrectly computes the modulus of a hierarchical data structure by using thesqrtof thedotproduct and returns aScalarField.
136-141: LGTM!The function
gradcorrectly computes the gradient of a hierarchical data structure and returns aVectorField.tests/simulator/refinement/test_2d_2_core.py (1)
146-147: Verify the necessity of twohierarchy_fromcalls.Ensure that both
EM_E.h5andEM_B.h5are required to be loaded separately and that this implementation is correct.tests/simulator/refinement/test_2d_10_core.py (1)
123-124: Verify the necessity of twohierarchy_fromcalls.Ensure that both
EM_E.h5andEM_B.h5are required to be loaded separately and that this implementation is correct.tests/functional/alfven_wave/alfven_wave1d.py (1)
164-164: Verify the sorting and extraction logic.Ensure that the sorting of patches and extraction of
xvalues fromB.level(1)is correct and necessary.pyphare/pyphare/core/box.py (4)
73-77: Verify the initialization logic innDBox.Ensure that the nested function
_getcorrectly initializeslowerandupperarrays and that thenDBoxobject is properly constructed.
81-82: Verify the initialization logic inBox1D.Ensure that calling the
nDBoxconstructor with dimension 1 correctly initializes a 1D box.
86-87: Verify the initialization logic inBox2D.Ensure that calling the
nDBoxconstructor with dimension 2 correctly initializes a 2D box.
91-92: Verify the initialization logic inBox3D.Ensure that calling the
nDBoxconstructor with dimension 3 correctly initializes a 3D box.pyphare/pyphare/pharesee/hierarchy/fromsim.py (5)
1-8: Imports look good.The imports are well-organized and correctly reference the necessary modules and classes.
13-20: Initial setup and assertions are correct.The initial setup correctly retrieves the necessary data from the simulator and performs appropriate assertions.
22-43: Logic for handling field quantities is correct.The code correctly handles field quantities, setting up patches and their data appropriately.
45-119: Logic for handling particles is well-implemented.The code correctly handles particle quantities, including error handling and setting up patches with particle data.
121-123: Patch levels and hierarchy are correctly constructed.The final construction of patch levels and the hierarchy is correct.
tests/simulator/test_run.py (3)
Line range hint
30-160:
Simulation setup and diagnostics configuration are correct.The configuration function correctly sets up the simulation parameters and diagnostics.
Line range hint
164-193:
Plot generation logic is correct.The function correctly generates plots for various quantities at different timestamps.
Line range hint
210-234:
Test methods are correctly implemented.The test methods correctly run simulations and verify the outputs.
tests/functional/tdtagged/td1dtagged.py (4)
Line range hint
70-108:
Simulation setup and diagnostics configuration are correct.The configuration function correctly sets up the simulation parameters and diagnostics.
Line range hint
130-224:
Figure generation logic is correct.The function correctly generates figures for tagged diagnostics.
266-266: Hierarchy retrieval logic is correct.The function correctly retrieves the hierarchy data for a given time.
Line range hint
269-274:
Post-advance logic is correct.The function correctly performs the post-advance actions for the simulation.
tests/simulator/test_diagnostics.py (5)
16-18: Imports look good.The imports are well-organized and correctly reference the necessary modules and classes.
Line range hint
110-110:
Test setup is correct.The setup method correctly initializes the test environment.
Line range hint
138-138:
Test teardown is correct.The teardown method correctly resets the test environment.
Line range hint
145-145:
Diagnostic dumping test is correct.The test correctly verifies the dumping of diagnostics.
Line range hint
153-153:
Diagnostic dump logic is correct.The core logic correctly verifies the diagnostic dumps.
pyphare/pyphare/pharesee/hierarchy/patchdata.py (10)
25-27: Deep copy implementation is correct.The
__deepcopy__method correctly avoids copying thedatasetattribute.
150-165: Correct calculation of ghost box and size.The calculation of
self.ghost_boxandself.sizebased on the centering is correct and ensures the correct handling of ghost cells.
36-46: Efficient caching of x coordinates.The property method for
xcaches the coordinates efficiently, avoiding redundant calculations.
48-58: Efficient caching of y coordinates.The property method for
ycaches the coordinates efficiently, avoiding redundant calculations.
60-70: Efficient caching of z coordinates.The property method for
zcaches the coordinates efficiently, avoiding redundant calculations.
83-110: Correct implementation of select and getitem methods.The
selectmethod correctly returns a view of the internal data based on the overlap of the input box, and the__getitem__method correctly delegates toselector directly accesses the dataset.
168-179: Correct implementation of meshgrid method.The
meshgridmethod correctly constructs a meshgrid based on the dimensions, and theselectparameter allows for efficient subsetting.
187-210: Correct initialization of ParticleData.The
__init__method correctly initializes theParticleDataobject, including handling different interpolation orders.
211-215: Correct implementation of select and getitem methods.The
selectmethod correctly returns the dataset for the given box, and the__getitem__method delegates toselect.
217-218: Correct implementation of size method.The
sizemethod correctly returns the size of the dataset.pyphare/pyphare/pharesee/hierarchy/scalarfield.py (14)
6-15: Correct initialization of ScalarField.The
__init__method correctly initializes theScalarFieldobject by callingcompute_hier_fromand passing the necessary parameters to the superclass initializer.
17-32: Correct implementation of add method.The
__add__method correctly handles addition with anotherScalarFieldor a scalar value, ensuring type safety and correctness.
34-35: Correct implementation of radd method.The
__radd__method correctly delegates to the__add__method.
37-52: Correct implementation of sub method.The
__sub__method correctly handles subtraction with anotherScalarFieldor a scalar value, ensuring type safety and correctness.
54-66: Correct implementation of mul method.The
__mul__method correctly handles multiplication with anotherScalarFieldor a scalar value, ensuring type safety and correctness.
68-69: Correct implementation of rmul method.The
__rmul__method correctly delegates to the__mul__method.
71-83: Correct implementation of truediv method.The
__truediv__method correctly handles division with anotherScalarFieldor a scalar value, ensuring type safety and correctness.
85-91: Correct implementation of rtruediv method.The
__rtruediv__method correctly handles division where theScalarFieldis the divisor and a scalar value is the dividend.
209-212: Correct implementation of neg method.The
__neg__method correctly handles negation of theScalarFieldby callingcompute_hier_fromwith_compute_neg.
93-111: Correct implementation of _compute_add method.The
_compute_addmethod correctly handles the addition of datasets from twoScalarFieldobjects or aScalarFieldand a scalar value.
113-131: Correct implementation of _compute_sub method.The
_compute_submethod correctly handles the subtraction of datasets from twoScalarFieldobjects or aScalarFieldand a scalar value.
133-161: Correct implementation of _compute_mul method.The
_compute_mulmethod correctly handles the multiplication of datasets from twoScalarFieldobjects or aScalarFieldand a scalar value.
163-191: Correct implementation of _compute_truediv method.The
_compute_truedivmethod correctly handles the division of datasets from twoScalarFieldobjects or aScalarFieldand a scalar value.
193-207: Correct implementation of _compute_rtruediv method.The
_compute_rtruedivmethod correctly handles the division where a scalar value is divided by the dataset of aScalarField.tests/functional/conservation/conserv.py (2)
134-137: Correct implementation of total_particles function.The
total_particlesfunction correctly iterates over particle levels and patches to calculate the total quantity using the provided callback function.
Line range hint
154-175:
Correct implementation of mag_energy function.The
mag_energyfunction correctly calculates the total magnetic energy on a given level by averagingByandBzontoBx.Tools
Ruff
157-157: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
tests/amr/data/field/coarsening/test_coarsen_field.py (1)
14-14: Correct import statement.The import statement for
FieldDatafrompyphare.pharesee.hierarchy.patchdatais correct and ensures the correct module is being used.pyphare/pyphare_tests/test_pharesee/__init__.py (4)
4-4: LGTM!The import statement for
Boxis correct and necessary.
10-10: LGTM!The import statement for
FieldDatais correct and necessary.
11-11: LGTM!The import statement for
ParticleDatais correct and necessary.
13-13: LGTM!The import statement for
Patchis correct and necessary.pyphare/pyphare/pharesee/run/run.py (1)
9-9: LGTM!The import statement for
listifyis correct and necessary.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1)
1-11: Imports are well-organized.Imports are correctly organized, ensuring necessary modules and packages are included.
tests/simulator/test_restarts.py (2)
22-23: Imports are well-organized.The new imports are correctly organized and necessary for the added functionalities.
201-201: Remove redundant check forid.The check for
id(pd0) != id(pd1)is redundant aspd0andpd1are different objects.- self.assertNotEqual(id(pd0), id(pd1))Likely invalid or redundant comment.
tests/amr/data/field/refine/test_refine_field.py (1)
6-6: Update Import Statement to Reflect New Module Structure.The import statement for
FieldDatahas been updated to import frompyphare.pharesee.hierarchy.patchdata. This change is consistent with the refactoring efforts mentioned in the PR objectives.pyphare/pyphare/pharesee/geometry.py (1)
5-6: Add Import Statements for New Functionalities.The import statements for
FieldDatafrompatchdataandis_root_lvlfromhierarchy_utilshave been added. These changes are consistent with the modular refactoring efforts mentioned in the PR objectives.tests/core/data/gridlayout/test_laplacian.py (1)
213-213: Adjust Slicing ofJxArray for Laplacian Computation.The slicing of the
Jxarray in thetest_laplacian_yee2Dfunction has been adjusted. Ensure that this change correctly reflects the intended Laplacian computation and that the test results are validated.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (8)
114-114: Simplify dictionary key check.Use
key in dictinstead ofkey in dict.keys().- if not are_compatible_hierarchies(hierarchies): + if not are_compatible_hierarchies(hierarchies):Likely invalid or redundant comment.
449-449: Ensure consistent handling of patch data.Verify that the
_compute_*functions handle all necessary cases consistently.Verification successful
Ensure consistent handling of patch data.
The
_compute_*functions across various files consistently handle patch data. The functions follow a similar structure and parameter usage, ensuring uniformity in their implementation.
vectorfield.pyutils.pyhierarchy_utils.pyscalarfield.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of `_compute_*` functions. # Test: Search for the function usage. Expect: Consistent handling of patch data in all cases. rg --type python -A 5 $'_compute_*'Length of output: 67
Script:
#!/bin/bash # Description: Verify the consistency of `_compute_*` functions. # Test: Search for function definitions and their usage. Expect: Consistent handling of patch data in all cases. rg --type py -A 5 '_compute_*'Length of output: 22405
149-149: Ensure consistent handling of patch data.Verify that the creation of new patch data handles all necessary cases consistently.
Verification successful
Ensure consistent handling of patch data.
The
new_patchdatas_fromfunction consistently handles the creation of new patch data by generatingFieldDataobjects and storing them in a dictionary. This pattern is evident both in its definition and usage.
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py: Function definition and usage confirm the consistent handling of patch data.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of new patch data creation. # Test: Search for the function usage. Expect: Consistent handling of patch data in all cases. rg --type python -A 5 $'new_patchdatas_from'Length of output: 74
Script:
#!/bin/bash # Description: Verify the consistency of new patch data creation. # Test: Search for the function usage. Expect: Consistent handling of patch data in all cases. rg --type py -A 5 $'new_patchdatas_from'Length of output: 1214
140-140: Ensure consistent handling of patch data.Verify that the patch data extraction handles all necessary cases consistently.
163-163: Ensure consistent handling of patch data.Verify that the creation of new patches handles all necessary cases consistently.
Verification successful
Verified: Consistent handling of patch data in new patch creation.
The
new_patches_fromfunction systematically extracts, computes, and appends patch data, ensuring consistency in handling.
- The function uses
extract_patchdatasto gather data.- It then uses
new_patchdatas_fromto compute new data.- Finally, it appends the new patches to the list.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of new patch creation. # Test: Search for the function usage. Expect: Consistent handling of patch data in all cases. rg --type python -A 5 $'new_patches_from'Length of output: 71
Script:
#!/bin/bash # Description: Verify the consistency of new patch creation. # Test: Search for the function usage. Expect: Consistent handling of patch data in all cases. rg --glob '*.py' -A 5 $'new_patches_from'Length of output: 1201
Script:
#!/bin/bash # Extract the full implementation of the `new_patches_from` function for detailed analysis. ast-grep --lang python --pattern $'def new_patches_from($_, $_, $_, $_, **$_):\n $$$'Length of output: 1481
373-373: Ensure consistent handling of field data.Verify that the flattening of 2D field data handles all necessary cases consistently.
Verification successful
Consistent handling of 2D field data verified.
The
flat_finest_field_2dfunction consistently handles 2D field data by properly managing ghost nodes, flattening data, and addressing overlaps.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of 2D field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type python -A 5 $'flat_finest_field_2d'Length of output: 75
Script:
#!/bin/bash # Description: Verify the consistency of 2D field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type py -A 5 $'flat_finest_field_2d'Length of output: 1142
Script:
#!/bin/bash # Description: Extract the full implementation of the `flat_finest_field_2d` function to review its consistency in handling 2D field data. # Extract the full implementation of `flat_finest_field_2d` ast-grep --lang python --pattern $'def flat_finest_field_2d($_, $_, $_):\n $$$' pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.pyLength of output: 5270
313-313: Ensure consistent handling of field data.Verify that the flattening of field data handles all necessary cases consistently.
Verification successful
Ensure consistent handling of field data.
The
flat_finest_fieldfunction is used consistently to handle field data across different parts of the codebase, ensuring reliable extraction and flattening of data.
tests/functional/alfven_wave/alfven_wave1d.pypyphare/pyphare/pharesee/run/run.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type python -A 5 $'flat_finest_field'Length of output: 72
Script:
#!/bin/bash # Description: Verify the consistency of field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type py -A 5 $'flat_finest_field'Length of output: 6635
Script:
#!/bin/bash # Description: Gather context for the usage of flat_finest_field in alfven_wave1d.py and run.py # Extract lines around the function calls in alfven_wave1d.py rg --type py -A 10 -B 10 $'flat_finest_field' tests/functional/alfven_wave/alfven_wave1d.py # Extract lines around the function calls in run.py rg --type py -A 10 -B 10 $'flat_finest_field' pyphare/pyphare/pharesee/run/run.pyLength of output: 4531
333-333: Ensure consistent handling of field data.Verify that the flattening of 1D field data handles all necessary cases consistently.
Verification successful
The handling of field data in
flat_finest_field_1dis consistent with other dimensions.The functions
flat_finest_field_1dandflat_finest_field_2ddemonstrate a consistent approach to flattening field data by removing ghost nodes to avoid overlapping and ensure no extrapolation for the interpolator. The logic applied is similar for both 1D and 2D cases, indicating a consistent handling of field data.
flat_finest_field_1dimplementation starts at line 333 inpyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py.flat_finest_field_2dimplementation is similar and handles 2D data with the same logic.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the consistency of 1D field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type python -A 5 $'flat_finest_field_1d'Length of output: 75
Script:
#!/bin/bash # Description: Verify the consistency of 1D field data flattening. # Test: Search for the function usage. Expect: Consistent handling of field data in all cases. rg --type py -A 5 $'flat_finest_field_1d'Length of output: 1138
Script:
#!/bin/bash # Extract the implementations of flat_finest_field_1d and flat_finest_field_2d from hierarchy_utils.py for detailed inspection. ast-grep --lang python --pattern $'def flat_finest_field_1d($_, $_, $_) { $$$ }' ast-grep --lang python --pattern $'def flat_finest_field_2d($_, $_, $_) { $$$ }'Length of output: 1105
Script:
#!/bin/bash # Extract the implementations of flat_finest_field_1d and flat_finest_field_2d from hierarchy_utils.py for detailed inspection. ast-grep --lang python --pattern 'def flat_finest_field_1d(hierarchy, qty, time=None):' ast-grep --lang python --pattern 'def flat_finest_field_2d(hierarchy, qty, time=None):'Length of output: 179
Script:
#!/bin/bash # Search for the implementations of flat_finest_field_1d and flat_finest_field_2d in hierarchy_utils.py with additional context. rg 'def flat_finest_field_1d' -A 20 pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py rg 'def flat_finest_field_2d' -A 20 pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.pyLength of output: 1906
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (1)
290-290: LGTM!The code changes are approved.
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (10)
675-742: LGTM!The code changes are approved.
307-324: LGTM!The code changes are approved.
553-558: LGTM!The code changes are approved.
Tools
GitHub Check: CodeQL
[notice] 553-553: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
399-453: LGTM!The code changes are approved.
336-350: LGTM!The code changes are approved.
351-355: LGTM!The code changes are approved.
356-397: LGTM!The code changes are approved.
228-236: LGTM!The code changes are approved.
12-15: Simplify dictionary key check.Use
key not in dictinstead ofkey not in dict.keys()for better readability and performance.- if isinstance(timestamp, str): + if timestamp in dict:Likely invalid or redundant comment.
559-610: Verify for cyclic imports.Ensure that the import statement does not cause a cyclic import.
Verification successful
No cyclic imports found.
The import statement in the
dist_plotmethod does not cause any cyclic imports based on the current codebase analysis.
pyphare.pharesee.plottingis used in tests and documentation, but no cyclic dependencies were detected.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the import statement does not cause a cyclic import. # Test: Check for cyclic imports. Expect: No cyclic imports. rg --type python 'import pyphare.pharesee.plotting'Length of output: 84
Script:
#!/bin/bash # Description: Verify that the import statement does not cause a cyclic import. # Test: Check for cyclic imports. Expect: No cyclic imports. rg 'import pyphare.pharesee.plotting' -A 5Length of output: 44
Script:
#!/bin/bash # Description: Verify that the import statement does not cause a cyclic import. # Test: Check for all instances of pyphare.pharesee.plotting across the codebase. Expect: No cyclic imports. rg 'pyphare.pharesee.plotting' -A 5Length of output: 935
Tools
Ruff
593-593: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
GitHub Check: CodeQL
[notice] 565-565: Cyclic import
Import of module pyphare.pharesee.plotting begins an import cycle.
[notice] 563-563: Module is imported with 'import' and 'import from'
Module 'copy' is imported with both 'import' and 'import from'.pyphare/pyphare/pharein/simulation.py (9)
329-329: LGTM!The logic for checking overlapping refinement boxes is sound.
349-350: LGTM!The logic for validating the refined box size against the smallest patch size is sound.
555-555: LGTM!The logic for raising an exception for negative resistivity values is sound.
563-563: LGTM!The logic for raising an exception for negative hyper-resistivity values is sound.
672-672: LGTM!The assertion for
max_nbr_levelsensures that it is set whenrefinementis not "boxes".
935-935: LGTM!The import of the
dillmodule is necessary for serialization.
936-936: LGTM!The import of the
codecsmodule is necessary for encoding the serialized data.
942-942: LGTM!The import of the
dillmodule is necessary for deserialization.
943-943: LGTM!The import of the
codecsmodule is necessary for decoding the serialized data.tests/simulator/test_advance.py (3)
20-22: LGTM! Import changes are consistent with refactoring objectives.The imports from
pyphare.pharesee.hierarchyandpyphare.pharesee.hierarchy.hierarchyare correct and necessary for the refactored functions.
460-460: LGTM! Correct usage offormat_timestamp.The function correctly uses
format_timestampto format timestamps.
562-562: LGTM! Correct usage offormat_timestamp.The function correctly uses
format_timestampto format timestamps.
There was a problem hiding this comment.
Actionable comments posted: 39
Outside diff range, codebase verification and nitpick comments (11)
pyphare/pyphare/pharesee/hierarchy/patchdata.py (3)
14-14: Suggestion: Improve the docstring for the__init__method.The docstring can be expanded to provide more details about the parameters and their expected types.
- """ - :param layout: a GridLayout representing the domain on which the data - is defined - :param quantity: ['field', 'particle'] - """ + """ + Initialize PatchData with layout and quantity. + :param layout: GridLayout representing the domain on which the data is defined + :param quantity: str, either 'field' or 'particle' + """
83-105: Enhancement: Improve theselectmethod's docstring.The docstring can be expanded to provide more details about the method's functionality and parameters.
- """ - return view of internal data based on overlap of input box - returns a view +1 in size in primal directions - """ + """ + Return a view of internal data based on the overlap of the input box. + :param box: Box object representing the region to select + :return: view of the internal data + """
187-196: Suggestion: Improve the docstring for the__init__method.The docstring can be expanded to provide more details about the parameters and their expected types.
- """ - :param layout: A GridLayout object representing the domain in which particles are - :param data: dataset containing particles - """ + """ + Initialize ParticleData with layout, data, and population name. + :param layout: GridLayout object representing the domain in which particles are + :param data: dataset containing particles + :param pop_name: str, name of the particle population + """pyphare/pyphare/pharesee/geometry.py (1)
5-5: Add a newline between standard library and local imports.To improve readability, add a newline between the standard library imports and local imports.
import numpy as np + from ..core import box as boxm from pyphare.core.box import Box from .hierarchy.patchdata import FieldData from .hierarchy.hierarchy_utils import is_root_lvlpyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1)
50-52: Rename unused loop control variables.The loop control variables
time,ilvl, andipare not used within the loop body. Rename them to_time,_ilvl, and_ipto indicate they are unused.for _time, patch_levels in hierarchy.time_hier.items(): for _ilvl, plvl in patch_levels.items(): for _ip, patch in enumerate(plvl.patches):Tools
Ruff
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (2)
117-117: Rename unused loop variable.The loop control variable
lvlis not used within the loop body. Rename it to_lvlfor clarity.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Rename unused loop variable.The loop control variable
lvlis not used within the loop body. Rename it to_lvlfor clarity.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
tests/functional/conservation/conserv.py (3)
Line range hint
154-175:
Improve readability and maintainability.Consider using a list comprehension to improve readability and maintainability.
- for ilvl, lvl in B.levels().items(): - if lvlNbr == ilvl: - tot = 0.0 - for ip, patch in enumerate(lvl.patches): - pdata = patch.patch_datas["Bx"] - ghosts_nbr = pdata.ghosts_nbr[0] - bx = patch.patch_datas["Bx"].dataset[ghosts_nbr:-ghosts_nbr] - bytmp = patch.patch_datas["By"].dataset[ghosts_nbr - 1 : -(ghosts_nbr - 1)] - bztmp = patch.patch_datas["Bz"].dataset[ghosts_nbr - 1 : -(ghosts_nbr - 1)] - by = 0.5 * (bytmp[1:] + bytmp[:-1]) - bz = 0.5 * (bztmp[1:] + bztmp[:-1]) - per_patch = np.sum((bx**2 + by**2 + bz**2) * 0.5 * pdata.layout.dl[0]) - tot += per_patch - return tot + tot = sum( + np.sum((bx**2 + by**2 + bz**2) * 0.5 * pdata.layout.dl[0]) + for ilvl, lvl in B.levels().items() if lvlNbr == ilvl + for patch in lvl.patches + for pdata in [patch.patch_datas["Bx"]] + for ghosts_nbr in [pdata.ghosts_nbr[0]] + for bx in [patch.patch_datas["Bx"].dataset[ghosts_nbr:-ghosts_nbr]] + for bytmp in [patch.patch_datas["By"].dataset[ghosts_nbr - 1 : -(ghosts_nbr - 1)]] + for bztmp in [patch.patch_datas["Bz"].dataset[ghosts_nbr - 1 : -(ghosts_nbr - 1)]] + for by in [0.5 * (bytmp[1:] + bytmp[:-1])] + for bz in [0.5 * (bztmp[1:] + bztmp[:-1])] + ) + return totTools
Ruff
157-157: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
Line range hint
178-190:
Improve readability and maintainability.Consider using a list comprehension to improve readability and maintainability.
- Bnrj = np.zeros_like(times) - K = np.zeros_like(times) - for it, t in enumerate(times): - B = r.GetB(t) - protons = r.GetParticles(t, "protons") - Bnrj[it] = mag_energy(B) - K[it] = total_kinetic(protons, kind=kkind) + Bnrj, K = zip(*[ + (mag_energy(r.GetB(t)), total_kinetic(r.GetParticles(t, "protons"), kind=kkind)) + for t in times + ]) + Bnrj, K = np.array(Bnrj), np.array(K)
Line range hint
178-190:
Improve readability and maintainability.Consider using a list comprehension to improve readability and maintainability.
- return [np.argmin(np.abs(times - t)) for t in (t1, t2)] + return [np.argmin(np.abs(times - t)) for t in (t1, t2)]pyphare/pyphare_tests/test_pharesee/__init__.py (1)
Line range hint
22-43:
Improve readability and maintainability.Consider using a list comprehension to improve readability and maintainability.
- ndim = ghost_box.ndim - dl, origin = layout.dl, layout.origin - xyz, directions = [], ["x", "y", "z"][:ndim] - for dimdex, direction in enumerate(directions): - primal = yee_element_is_primal(qty, direction) - nbrGhosts = layout.nbrGhosts(primal, layout.interp_order) - xyz.append( - origin[dimdex] - + np.arange(ghost_box.shape[dimdex] + primal) * dl[dimdex] - - nbrGhosts * dl[dimdex] - ) + ndim = ghost_box.ndim + dl, origin = layout.dl, layout.origin + directions = ["x", "y", "z"][:ndim] + xyz = [ + origin[dimdex] + + np.arange(ghost_box.shape[dimdex] + yee_element_is_primal(qty, direction)) * dl[dimdex] + - layout.nbrGhosts(yee_element_is_primal(qty, direction), layout.interp_order) * dl[dimdex] + for dimdex, direction in enumerate(directions) + ]
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (47)
- pyphare/pyphare/core/box.py (1 hunks)
- pyphare/pyphare/core/operators.py (1 hunks)
- pyphare/pyphare/core/phare_utilities.py (1 hunks)
- pyphare/pyphare/cpp/init.py (1 hunks)
- pyphare/pyphare/pharein/init.py (2 hunks)
- pyphare/pyphare/pharein/diagnostics.py (2 hunks)
- pyphare/pyphare/pharein/examples/job.py (8 hunks)
- pyphare/pyphare/pharein/maxwellian_fluid_model.py (1 hunks)
- pyphare/pyphare/pharein/restarts.py (2 hunks)
- pyphare/pyphare/pharein/simulation.py (5 hunks)
- pyphare/pyphare/pharesee/geometry.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/init.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromh5.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromsim.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patch.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchlevel.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/scalarfield.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/vectorfield.py (1 hunks)
- pyphare/pyphare/pharesee/particles.py (1 hunks)
- pyphare/pyphare/pharesee/plotting.py (1 hunks)
- pyphare/pyphare/pharesee/run/init.py (1 hunks)
- pyphare/pyphare/pharesee/run/run.py (1 hunks)
- pyphare/pyphare/pharesee/run/utils.py (4 hunks)
- pyphare/pyphare/simulator/simulator.py (1 hunks)
- pyphare/pyphare_tests/test_pharesee/CMakeLists.txt (1 hunks)
- pyphare/pyphare_tests/test_pharesee/init.py (3 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry.py (2 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (6 hunks)
- pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1 hunks)
- tests/amr/data/field/coarsening/test_coarsen_field.py (1 hunks)
- tests/amr/data/field/refine/test_refine_field.py (1 hunks)
- tests/core/data/gridlayout/test_laplacian.py (1 hunks)
- tests/functional/alfven_wave/alfven_wave1d.py (1 hunks)
- tests/functional/conservation/conserv.py (3 hunks)
- tests/functional/tdtagged/td1dtagged.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 (6 hunks)
- tests/simulator/test_diagnostic_timestamps.py (2 hunks)
- tests/simulator/test_diagnostics.py (1 hunks)
- tests/simulator/test_initialization.py (2 hunks)
- tests/simulator/test_restarts.py (4 hunks)
- tests/simulator/test_run.py (6 hunks)
- tools/python3/phloping.py (1 hunks)
Files skipped from review due to trivial changes (11)
- pyphare/pyphare/core/box.py
- pyphare/pyphare/core/phare_utilities.py
- pyphare/pyphare/cpp/init.py
- pyphare/pyphare/pharein/diagnostics.py
- pyphare/pyphare/pharein/maxwellian_fluid_model.py
- pyphare/pyphare/pharesee/particles.py
- pyphare/pyphare/pharesee/run/init.py
- pyphare/pyphare/simulator/simulator.py
- pyphare/pyphare_tests/test_pharesee/CMakeLists.txt
- tests/amr/data/field/coarsening/test_coarsen_field.py
- tests/amr/data/field/refine/test_refine_field.py
Additional context used
Learnings (1)
tests/simulator/test_restarts.py (1)
Learnt from: PhilipDeegan PR: PHAREHUB/PHARE#784 File: tests/simulator/test_restarts.py:333-339 Timestamp: 2023-11-25T19:21:40.002Z Learning: The user has indicated that minimal changes were made to get the function working, but acknowledges that there may still be redundant code present, such as sleep/wait/advances. The user is open to further streamlining the code.
GitHub Check: CodeQL
pyphare/pyphare/pharesee/hierarchy/patch.py
[notice] 49-49: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/vectorfield.py
[notice] 85-85: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[notice] 57-57: Unused local variable
Variable self is not used.
[notice] 58-58: Unused local variable
Variable other is not used.
[notice] 80-80: Unused local variable
Variable self is not used.
[notice] 81-81: Unused local variable
Variable other is not used.pyphare/pyphare/pharesee/hierarchy/patchdata.py
[warning] 123-123: Overwriting attribute in super-class or sub-class
Assignment overwrites attribute layout, which was previously defined in superclass PatchData.pyphare/pyphare/pharesee/plotting.py
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.pyphare/pyphare/pharesee/hierarchy/fromh5.py
[notice] 8-8: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 9-9: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 17-17: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy_utils begins an import cycle.pyphare/pyphare/pharesee/run/run.py
[notice] 261-261: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
[notice] 38-38: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 236-236: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 12-12: Unused import
Import of '_current1d' is not used.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
[notice] 187-187: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 193-193: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.
[notice] 230-230: Imprecise assert
assertTrue(a < b) cannot provide an informative message. Using assertLess(a, b) instead will give more informative messages.pyphare/pyphare_tests/test_pharesee/test_geometry.py
[notice] 295-295: Imprecise assert
assertTrue(a is b) cannot provide an informative message. Using assertIs(a, b) instead will give more informative messages.pyphare/pyphare/pharesee/run/utils.py
[notice] 349-349: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
[notice] 1-1: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.pyphare/pyphare/pharesee/hierarchy/hierarchy.py
[notice] 553-553: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[warning] 143-143: Comparison of identical values
Comparison of identical values; use cmath.isnan() if testing for not-a-number.
[failure] 186-186: Potentially uninitialized local variable
Local variable 'slice_dim' may be used before it is initialized.
[failure] 187-187: Potentially uninitialized local variable
Local variable 'cst_dim' may be used before it is initialized.
[notice] 565-565: Cyclic import
Import of module pyphare.pharesee.plotting begins an import cycle.
[notice] 563-563: Module is imported with 'import' and 'import from'
Module 'copy' is imported with both 'import' and 'import from'.
Ruff
tools/python3/phloping.py
53-53: SyntaxError: Unexpected indentation
pyphare/pyphare/pharesee/hierarchy/fromh5.py
27-27: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
98-98: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
246-249: Use ternary operator
selection_box = hier.selection_box[time] if hier.selection_box is not None else Noneinstead ofif-else-blockReplace
if-else-block withselection_box = hier.selection_box[time] if hier.selection_box is not None else None(SIM108)
320-320: Do not
assert False(python -Oremoves these calls), raiseAssertionError()Replace
assert False(B011)
pyphare/pyphare/pharesee/run/run.py
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
54-54: Ambiguous variable name:
l(E741)
tests/simulator/test_restarts.py
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
pyphare/pyphare/pharesee/run/utils.py
264-269: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
91-91: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
26-26: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
245-245: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
246-246: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
270-270: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
593-593: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
626-626: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
638-638: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
tests/simulator/test_advance.py
362-362: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
Additional comments not posted (123)
pyphare/pyphare/pharesee/hierarchy/patchlevel.py (2)
4-6: LGTM! Initialization is straightforward.The
__init__method correctly initializes thelevel_numberandpatchesattributes.
8-9: LGTM! Iterator implementation is correct.The
__iter__method correctly returns an iterator for thepatchesattribute.pyphare/pyphare/pharesee/hierarchy/__init__.py (2)
1-4: LGTM! Import statements are correct.The import statements are necessary and correctly included.
13-42: LGTM!hierarchy_fromfunction is well-implemented.The function is well-documented and handles different scenarios correctly.
pyphare/pyphare/pharesee/hierarchy/patch.py (3)
34-35: LGTM!__str__method is well-implemented.The method provides a useful string representation of the
Patchobject.
37-38: LGTM!__repr__method is correctly implemented.The method correctly returns the same string representation as the
__str__method.
40-44: LGTM!copymethod is well-implemented.The method correctly returns a deep copy of the
Patchobject without copyingpatchdatas.datasets.pyphare/pyphare/pharein/restarts.py (1)
Line range hint
63-63:
LGTM! Theconserve_existingfunction is well-implemented.The function correctly trims timestamps if files exist for that time and mode is "conserve".
pyphare/pyphare/pharesee/hierarchy/vectorfield.py (3)
15-29: Review the initializer ofVectorField.The initializer (
__init__) properly initializes theVectorFieldby renaming hierarchy elements and calling the superclass initializer. This looks good and well-structured.
31-34: Review the__mul__method.The
__mul__method correctly handles multiplication with a scalar and returns a newVectorField. This looks good.
36-37: Review the__rmul__method.The
__rmul__method simply calls__mul__which is appropriate for handling reverse multiplication.pyphare/pyphare/pharein/examples/job.py (4)
3-4: Review the import statements.The import statements look good and necessary for the script.
11-11: Review theSimulationconfiguration.The
Simulationconfiguration is well-structured and the comments provide good explanations.
43-43: Review thebxfunction.The
bxfunction correctly calculates the magnetic field and retrieves the simulation domain.
Line range hint
47-105:
Review the diagnostics and model configurations.The diagnostic and model configurations are well-defined and appropriate for the simulation.
pyphare/pyphare/core/operators.py (10)
1-1: Review the import statements.The import statements are appropriate and necessary for the operations.
8-19: Review the_compute_dot_productfunction.The function correctly computes the dot product of two vector fields.
22-29: Review the_compute_sqrtfunction.The function correctly computes the square root of a scalar field.
32-52: Review the_compute_cross_productfunction.The function correctly computes the cross product of two vector fields.
55-80: Review the_compute_gradfunction.The function computes the gradient of a scalar field but only handles 2D cases. The TODO comment indicates the need for handling 3D cases.
83-99: Review thedotfunction.The function handles the dot product of two
VectorFieldinstances and returns aScalarField.
102-118: Review thecrossfunction.The function handles the cross product of two
VectorFieldinstances and returns aVectorField.
121-127: Review thesqrtfunction.The function handles the square root of a
ScalarFieldand returns aScalarField.
130-133: Review themodulusfunction.The function correctly computes the modulus of a
VectorFieldby taking the square root of the dot product.
136-141: Review thegradfunction.The function computes the gradient of a
ScalarFieldand returns aVectorField.tools/python3/phloping.py (1)
41-42: Review the import statement within_particlesmethod.The import statement is necessary for obtaining times from HDF5 files.
tests/simulator/refinement/test_2d_2_core.py (2)
146-146: Verify the parameters forhierarchy_from.Ensure that
path,time, anddatahierare correctly set and appropriate for the function call.
147-147: Verify the parameters forhierarchy_from.Ensure that
path,time, anddatahierare correctly set and appropriate for the function call.tests/simulator/refinement/test_2d_10_core.py (2)
123-123: Verify the parameters forhierarchy_from.Ensure that
path,time, anddatahierare correctly set and appropriate for the function call.
124-124: Verify the parameters forhierarchy_from.Ensure that
path,time, anddatahierare correctly set and appropriate for the function call.tests/functional/alfven_wave/alfven_wave1d.py (1)
164-164: Verify the sorting logic and data access.Ensure that the patches are correctly sorted based on their lower bound and that the access to the
Bydata is appropriate.tests/simulator/test_diagnostic_timestamps.py (2)
15-15: Verify the necessity and usage of thehierarchy_fromimport.Ensure that the function is used appropriately in the file and that the import is necessary.
181-181: Verify the correctness of theformat_timestampfunction usage.Ensure that the formatted timestamps match the expected format and that the function is used correctly.
pyphare/pyphare/pharesee/hierarchy/fromsim.py (5)
13-20: Ensure domain_box dimensions match.The assertion checks that the dimensions of the domain box match those of the simulator's domain box. This is a good practice to prevent dimension mismatches.
22-43: Process field quantities correctly.The code correctly processes field quantities by creating patches with field data. Ensure that
field_qtiesandGridLayoutare correctly defined and imported.
45-116: Handle particle quantities with care.The code handles particle quantities by creating patches with particle data. Ensure that the population name is correctly specified and that the particle data is correctly reshaped and added to the patches.
118-119: Raise appropriate errors for invalid quantities.The code raises a
ValueErrorif an invalid quantity is specified. This is a good practice to ensure that only valid quantities are processed.
121-123: Return the PatchHierarchy object.The function returns a
PatchHierarchyobject containing all the processed patch levels. Ensure thatPatchHierarchyis correctly defined and imported.tests/simulator/test_run.py (5)
23-30: Ensure directories are created.The code correctly ensures that the diagnostic and plot directories are created if they do not already exist.
158-160: Add population name to FluidDiagnostics.The
FluidDiagnosticssetup now includes the population name. This ensures that the diagnostics are correctly associated with the specified population.
179-182: Generate plots for diagnostic quantities.The code correctly generates plots for various diagnostic quantities at specified timestamps.
205-208: Check file existence and size.The code correctly checks that the specified file exists and has a size of at least the specified minimum. This ensures that the file is not missing or corrupted.
231-234: Check plot file existence for diagnostic quantities.The code correctly checks that the plot files for various diagnostic quantities exist and have the expected size.
tests/functional/tdtagged/td1dtagged.py (1)
266-266: Retrieve hierarchical data from HDF5 file.The code correctly retrieves hierarchical data from a specified HDF5 file at a specified time. Ensure that the
hierarchy_fromfunction is correctly defined and imported.tests/simulator/test_diagnostics.py (1)
16-18: Ensure correct imports for hierarchy functions.The code correctly imports functions for handling hierarchical data from HDF5 files.
pyphare/pyphare/pharesee/hierarchy/patchdata.py (4)
25-27: LGTM!The
__deepcopy__method correctly uses thedeep_copyutility to avoid copying certain attributes.
75-81: LGTM!The
__str__and__repr__methods provide clear string representations of theFieldDataobject.
168-179: LGTM!The
meshgridmethod correctly generates a meshgrid based on theFieldDatadimensions.
211-213: LGTM!The
selectmethod correctly returns the dataset for the given box.pyphare/pyphare/pharesee/hierarchy/scalarfield.py (13)
17-32: LGTM!The
__add__method correctly implements addition forScalarFieldobjects and scalars.
34-36: LGTM!The
__radd__method correctly uses the__add__method for right-side addition.
37-52: LGTM!The
__sub__method correctly implements subtraction forScalarFieldobjects and scalars.
54-66: LGTM!The
__mul__method correctly implements multiplication forScalarFieldobjects and scalars.
68-70: LGTM!The
__rmul__method correctly uses the__mul__method for right-side multiplication.
71-83: LGTM!The
__truediv__method correctly implements true division forScalarFieldobjects and scalars.
85-91: LGTM!The
__rtruediv__method correctly implements right-side true division for scalars.
93-111: LGTM!The
_compute_addmethod correctly computes the addition of patch data.
113-131: LGTM!The
_compute_submethod correctly computes the subtraction of patch data.
133-161: LGTM!The
_compute_mulmethod correctly computes the multiplication of patch data.
163-191: LGTM!The
_compute_truedivmethod correctly computes the true division of patch data.
193-207: LGTM!The
_compute_rtruedivmethod correctly computes the right-side true division of patch data.
209-212: LGTM!The
__neg__method correctly computes the negation of theScalarField.pyphare/pyphare/pharesee/plotting.py (3)
Line range hint
1-138: LGTM!The
dist_plotfunction is well-structured and provides extensive customization options for plotting the phase space of particles.Tools
GitHub Check: CodeQL
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.
Line range hint
139-155: LGTM!The
connect_bboxfunction effectively connects two bounding boxes with lines and patches.Tools
GitHub Check: CodeQL
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.
Line range hint
156-193: LGTM!The
zoom_effectfunction effectively connects two axes with a zoom effect.Tools
GitHub Check: CodeQL
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.pyphare/pyphare/pharesee/hierarchy/fromh5.py (14)
37-41: LGTM!The
make_layoutfunction correctly creates aGridLayoutobject.
44-45: LGTM!The
is_pop_fluid_filefunction correctly checks if a file is a population fluid file.
48-56: LGTM!The
particle_dataset_namefunction correctly returns the dataset name for a given particle file.
59-60: LGTM!The
is_particle_filefunction correctly checks if a file is a particle file.
63-64: LGTM!The
pop_namefunction correctly returns the population name for a given file.
123-124: LGTM!The
patch_has_datasetsfunction correctly checks if the HDF5 patch group has datasets.
127-129: LGTM!The
h5_filename_fromfunction correctly returns the HDF5 filename from diagnostic information.
132-139: LGTM!The
get_times_from_h5function correctly retrieves times from an HDF5 file.
142-143: LGTM!The
create_from_all_timesfunction correctly checks if a new hierarchy should be created from all times.
146-147: LGTM!The
create_from_timesfunction correctly checks if a new hierarchy should be created from specified times.
150-151: LGTM!The
load_all_timesfunction correctly checks if all times should be loaded into an existing hierarchy.
154-155: LGTM!The
load_one_timefunction correctly checks if a single time should be loaded into an existing hierarchy.
158-213: LGTM!The
patch_levels_from_h5function correctly creates a dictionary of PatchLevels from a given time in an HDF5 file.
233-256: LGTM!The
add_data_from_h5function correctly adds new PatchDatas to an existing PatchHierarchy for an existing time.Tools
Ruff
246-249: Use ternary operator
selection_box = hier.selection_box[time] if hier.selection_box is not None else Noneinstead ofif-else-blockReplace
if-else-block withselection_box = hier.selection_box[time] if hier.selection_box is not None else None(SIM108)
pyphare/pyphare/pharein/__init__.py (2)
7-21: Import changes look good.The added import statements are consistent and necessary for the module's functionality.
23-33: Changes to__all__list look good.The updated
__all__list is consistent with the imports and ensures the correct entities are exported.tests/simulator/test_restarts.py (7)
168-171: Changes tocheck_diagsmethod look good.The changes improve the method by adding more checks and making it more robust.
201-202: Changes tocheck_diagsmethod look good.The additional checks for data types and dataset equality enhance the robustness of the method.
213-215: Changes tocheck_diagsmethod look good.The changes to count levels and patches ensure comprehensive validation.
168-171: Changes totest_restartsmethod look good.The changes ensure proper setup for the simulation and make the test more comprehensive.
168-171: Changes totest_restarts_elapsed_timemethod look good.The changes ensure proper setup for the simulation and make the test more comprehensive.
168-171: Changes totest_mode_conservemethod look good.The changes ensure proper setup for the simulation and make the test more comprehensive.
168-171: Changes totest_input_validation_trailing_slashmethod look good.The changes ensure proper setup for the simulation and make the test more comprehensive.
pyphare/pyphare/pharesee/run/utils.py (4)
128-136: Changes to_compute_currentmethod look good.The changes add support for 2D cases and ensure proper handling of different dimensions.
128-136: Changes to_compute_divBmethod look good.The changes add support for 2D cases and ensure proper handling of different dimensions.
128-136: Addition of_ppp_to_ppp_domain_slicingfunction looks good.The new function provides necessary domain slicing for primal-primal-primal centering on Yee grids.
128-136: Changes to_compute_to_primalmethod look good.The changes ensure proper handling of different centerings and fill ghost cells with correct values.
pyphare/pyphare/pharesee/geometry.py (1)
5-6: EnsureFieldDataandis_root_lvlare used in the file.Verify that these newly added imports are utilized in the file. If they are not used, consider removing them to keep the imports clean.
Verification successful
Both
FieldDataandis_root_lvlare used in the file.
FieldDatais used in anassertstatement.is_root_lvlis used in anifcondition.No further action is required regarding these imports.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if `FieldData` and `is_root_lvl` are used in the file. # Test: Search for the usage of `FieldData` and `is_root_lvl`. Expect: At least one occurrence each. rg --type python 'FieldData' pyphare/pyphare/pharesee/geometry.py rg --type python 'is_root_lvl' pyphare/pyphare/pharesee/geometry.pyLength of output: 195
Script:
#!/bin/bash # Description: Verify if `FieldData` and `is_root_lvl` are used in the file. # Test: Search for the usage of `FieldData` and `is_root_lvl`. Expect: At least one occurrence each. rg 'FieldData' pyphare/pyphare/pharesee/geometry.py rg 'is_root_lvl' pyphare/pyphare/pharesee/geometry.pyLength of output: 330
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (8)
85-105: LGTM!The function correctly identifies patches containing a given point and handles potential errors.
Tools
Ruff
91-91: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
108-131: LGTM!The function correctly applies the callback function to all patch data and constructs the new hierarchy properly.
134-142: LGTM!The function correctly extracts patch data from a list of hierarchies for a specific patch.
145-151: LGTM!The function correctly applies the compute function to create new patch data.
154-166: LGTM!The function correctly applies the compute function to create new patches from hierarchies.
169-170: LGTM!The function correctly checks if a patch level is the root level.
173-191: LGTM!The function correctly maps quantities to their respective getters.
194-215: LGTM!The function correctly checks if a quantity is a field quantity.
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (1)
290-290: LGTM!The code correctly asserts that boxes do not overlap with patch interiors.
tests/simulator/test_initialization.py (2)
19-19: Import looks good.The import statement for
merge_particlesfromhierarchy_utilsis correct.
20-20: Import looks good.The import statement for
hierarchy_fromfromhierarchyis correct.pyphare/pyphare/pharesee/hierarchy/hierarchy.py (12)
1-6: Imports look good.The import statements are correct and necessary for the functionality provided in this file.
12-15: Function looks good.The
format_timestampfunction correctly formats the timestamp based on its type.
95-102: Method looks good.The
nbytesmethod correctly calculates the total number of bytes used by the datasets in the hierarchy.
104-109: Method looks good.The
nbrPatchesmethod correctly calculates the total number of patches in the hierarchy.
205-208: Method looks good.The
finest_levelmethod correctly returns the finest level in the hierarchy.
210-213: Method looks good.The
levelsmethod correctly returns the levels for a given time.
262-265: Method looks good.The
quantitiesmethod correctly returns the quantities in the hierarchy if it is homogeneous.
267-279: Method looks good.The
global_minmethod correctly calculates the global minimum value of a quantity in the hierarchy.Tools
Ruff
270-270: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
281-293: Method looks good.The
global_maxmethod correctly calculates the global maximum value of a quantity in the hierarchy.
295-300: Method looks good.The
refined_domain_boxmethod correctly returns the refined domain box for a given level number.
302-305: Method looks good.The
level_domain_boxmethod correctly returns the domain box for a given level number.
228-236: Rename unused loop variables.The loop variables
timeandilvlare not used within the loop body. Rename them to_timeand_ilvlto avoid confusion.for _time, levels in self.time_hier.items(): new_lvls = {} for _ilvl, level in levels.items():Likely invalid or redundant comment.
tests/simulator/test_advance.py (3)
460-460: Ensure correct usage offormat_timestamp.Verify that
format_timestampis used correctly to format timestamps.
562-562: Ensure correct usage offormat_timestamp.Verify that
format_timestampis used correctly to format timestamps.
20-22: Verify the necessity of new imports.Ensure that the imported
hierarchy_from,format_timestamp, andmerge_particlesare used in the file.Verification successful
The newly imported functions are used in the file.
The imported functions
hierarchy_from,format_timestamp, andmerge_particlesare utilized in thetests/simulator/test_advance.pyfile, confirming their necessity.
hierarchy_fromis used multiple times.format_timestampis used in assertions and other logic.merge_particlesis used for particle hierarchy merging.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the newly imported functions. # Test: Search for the usage of the newly imported functions. Expect: Functions are used in the file. rg --type python -w 'hierarchy_from|format_timestamp|merge_particles' tests/simulator/test_advance.pyLength of output: 134
Script:
#!/bin/bash # Description: Verify the usage of the newly imported functions. # Test: Search for the usage of the newly imported functions. Expect: Functions are used in the file. rg -w 'hierarchy_from|format_timestamp|merge_particles' tests/simulator/test_advance.pyLength of output: 971
pyphare/pyphare/pharein/simulation.py (4)
349-350: Ensure correct usage of refined box dimensions.Verify that the changes to the loop control variable improve code clarity and correctness.
555-555: Ensure clear error message for negative resistivity.Verify that the error message for negative resistivity is clear and correctly formatted.
563-563: Ensure clear error message for negative hyper-resistivity.Verify that the error message for negative hyper-resistivity is clear and correctly formatted.
672-672: Ensure necessity and correctness of assertion formax_nbr_levels.Verify that the assertion for
max_nbr_levelsis necessary and correctly placed.
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1 hunks)
- pyphare/pyphare/simulator/simulator.py (2 hunks)
- tests/diagnostic/init.py (1 hunks)
- tests/simulator/test_restarts.py (4 hunks)
Files skipped from review due to trivial changes (1)
- tests/diagnostic/init.py
Files skipped from review as they are similar to previous changes (1)
- pyphare/pyphare/simulator/simulator.py
Additional context used
Ruff
tests/simulator/test_restarts.py
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
177-177: Local variable
n_quantities_per_patchis assigned to but never usedRemove assignment to unused variable
n_quantities_per_patch(F841)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
26-26: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
245-245: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
246-246: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
270-270: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
593-593: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
626-626: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
638-638: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
GitHub Check: CodeQL
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
[notice] 553-553: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[warning] 143-143: Comparison of identical values
Comparison of identical values; use cmath.isnan() if testing for not-a-number.
[failure] 186-186: Potentially uninitialized local variable
Local variable 'slice_dim' may be used before it is initialized.
[failure] 187-187: Potentially uninitialized local variable
Local variable 'cst_dim' may be used before it is initialized.
[notice] 565-565: Cyclic import
Import of module pyphare.pharesee.plotting begins an import cycle.
[notice] 563-563: Module is imported with 'import' and 'import from'
Module 'copy' is imported with both 'import' and 'import from'.
Additional comments not posted (27)
tests/simulator/test_restarts.py (4)
22-22: Import statement looks good.The addition of
ParticleDataimport is necessary for the functionality within the file.
23-23: Import statement looks good.The addition of
get_all_available_quantities_from_h5import is necessary for the functionality within the file.
199-200: LGTM!The added type check and comparison for
ParticleDatainstances are correct and necessary.
211-213: LGTM!The additions ensure the correct number of levels and patches are present, and the assertions validate the expected conditions.
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (23)
1-1: Import statement looks good.The addition of
Patchimport is necessary for the functionality within the file.
2-2: Import statement looks good.The addition of
PatchLevelimport is necessary for the functionality within the file.
3-3: Import statement looks good.The addition of
Boximport is necessary for the functionality within the file.
4-4: Import statement looks good.The addition of
boximport asboxmis necessary for the functionality within the file.
5-5: Import statement looks good.The addition of
refinement_ratioimport is necessary for the functionality within the file.
6-6: Import statement looks good.The addition of
listifyimport is necessary for the functionality within the file.
8-8: Import statement looks good.The addition of
numpyimport asnpis necessary for the functionality within the file.
9-9: Import statement looks good.The addition of
matplotlib.pyplotimport aspltis necessary for the functionality within the file.
12-15: Functionformat_timestamplooks good.The function formats a timestamp as a string and is necessary for handling timestamps within the file.
18-20: ClassPatchHierarchyand docstring look good.The class is a collection of patch levels, and the docstring provides a brief description.
30-31: LGTM!The check ensures that
timesis a list or tuple.
33-34: LGTM!The check ensures that
patch_levelsis a list or tuple.
36-43: LGTM!The code handles
selection_boxcorrectly by checking if it is a list or tuple and formatting it usingformat_timestamp.
45-45: LGTM!The assertion ensures consistency between
timesandpatch_levels.
47-67: LGTM!The initialization sets up the attributes for the
PatchHierarchyobject correctly.
69-69: LGTM!The call to
updateensures that thePatchHierarchyobject is updated after initialization.
71-72: LGTM!The
__getitem__method allows accessing attributes of thePatchHierarchyobject using dictionary-like syntax.
74-94: LGTM!The
updatemethod correctly updates thePatchHierarchyobject by creating new levels for each quantity.
95-103: LGTM!The
nbytesmethod correctly calculates the total number of bytes used by the datasets in thePatchHierarchyobject.
104-110: LGTM!The
nbrPatchesmethod correctly calculates the total number of patches in thePatchHierarchyobject.
111-132: LGTM!The
simproperty correctly handles the deserialization of the simulation from the data files.Tools
Ruff
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
133-200: LGTM!The
__call__method correctly implements the logic for extracting data slices from thePatchHierarchyobject.Tools
GitHub Check: CodeQL
[warning] 143-143: Comparison of identical values
Comparison of identical values; use cmath.isnan() if testing for not-a-number.
[failure] 186-186: Potentially uninitialized local variable
Local variable 'slice_dim' may be used before it is initialized.
[failure] 187-187: Potentially uninitialized local variable
Local variable 'cst_dim' may be used before it is initialized.
202-204: LGTM!The
_default_timemethod correctly returns the default time for thePatchHierarchyobject.
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (9)
- pyphare/pyphare/pharein/examples/job.py (7 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py (1 hunks)
- pyphare/pyphare/pharesee/run/run.py (1 hunks)
- pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1 hunks)
- src/amr/tagging/default_hybrid_tagger_strategy.hpp (1 hunks)
- tests/core/data/gridlayout/test_laplacian.py (1 hunks)
- tests/functional/alfven_wave/alfven_wave1d.py (2 hunks)
- tests/simulator/test_restarts.py (4 hunks)
- tools/python3/phloping.py (1 hunks)
Files skipped from review due to trivial changes (1)
- src/amr/tagging/default_hybrid_tagger_strategy.hpp
Files skipped from review as they are similar to previous changes (3)
- pyphare/pyphare/pharein/examples/job.py
- pyphare/pyphare/pharesee/hierarchy/patchdata.py
- tests/functional/alfven_wave/alfven_wave1d.py
Additional context used
Ruff
tools/python3/phloping.py
43-43: Local variable
bad_diag_erroris assigned to but never usedRemove assignment to unused variable
bad_diag_error(F841)
pyphare/pyphare/pharesee/run/run.py
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
54-54: Ambiguous variable name:
l(E741)
tests/simulator/test_restarts.py
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
GitHub Check: CodeQL
pyphare/pyphare/pharesee/run/run.py
[notice] 260-260: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
[notice] 38-38: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 235-235: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 12-12: Unused import
Import of '_current1d' is not used.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
[notice] 193-193: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.
Additional comments not posted (9)
tests/core/data/gridlayout/test_laplacian.py (1)
213-213: Review of the slicing operation intest_laplacian_yee2D.The slicing operation for
Jx_xappears to be correctly adjusted in the current code. The indices are correctly set to include the boundary conditions for the Laplacian computation. The previous issue with the misplaced comma has been addressed in this version of the code.pyphare/pyphare/pharesee/run/run.py (3)
4-7: Cyclic Import DetectedThe import statements from lines 4 and 5 introduce cyclic import issues. Consider refactoring to avoid cyclic imports. One approach could be moving common functions to a separate utility module.
12-12: Remove unused importThe import statement for
_current1dis unused and should be removed.Tools
Ruff
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
GitHub Check: CodeQL
[notice] 12-12: Unused import
Import of '_current1d' is not used.
47-51: Combine Nested If StatementsUse a single if statement instead of nested if statements.
Tools
Ruff
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (3)
26-26: Combine Import StatementsThe module
pyphare.phareinis imported with bothimportandimport from. Combine these imports to improve readability.Tools
GitHub Check: CodeQL
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.
54-54: Avoid Ambiguous Variable NameThe variable name
lis ambiguous and can be easily mistaken for the digit1or the letterI. Consider renaming it to something more descriptive.Tools
Ruff
54-54: Ambiguous variable name:
l(E741)
193-193: Use assertGreater for Better Error MessagesThe assert statement can be improved for better error messages.
Tools
GitHub Check: CodeQL
[notice] 193-193: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.tests/simulator/test_restarts.py (2)
160-160: Usekey in dictInstead ofkey in dict.keys()Remove
.keys()for better performance and readability.Tools
Ruff
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Avoid Variable ShadowingThe
timemodule imported at the beginning of the file is shadowed by the loop variabletime. Consider renaming the loop variable.Tools
Ruff
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tools/python3/phloping.py (1 hunks)
Additional context used
Ruff
tools/python3/phloping.py
43-43: Local variable
bad_diag_erroris assigned to but never usedRemove assignment to unused variable
bad_diag_error(F841)
Additional comments not posted (2)
tools/python3/phloping.py (2)
41-41: Import statement added.The import statement for
get_times_from_h5has been added. This is necessary for retrieving times from theparticle_count.h5file.
49-50: Initializeparticles_per_level_per_time_stepandpcount_hier.The initialization of
particles_per_level_per_time_stepandpcount_hieris clear and straightforward.
nicolasaunai
force-pushed
the
refac_hierarchies
branch
from
bcc0caa to
bd95bb0
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- tools/python3/phloping.py (1 hunks)
Additional context used
Ruff
tools/python3/phloping.py
43-43: Local variable
bad_diag_erroris assigned to but never usedRemove assignment to unused variable
bad_diag_error(F841)
Additional comments not posted (4)
tools/python3/phloping.py (4)
41-41: Import statement added.The import statement for
get_times_from_h5is correctly added.
47-47: Ensureget_times_from_h5handles errors.Ensure that
get_times_from_h5handles errors gracefully, such as when the file does not exist or is corrupted.
42-42: Remove unused variable.The variable
bad_diag_erroris assigned but never used. It should be removed to clean up the code.- bad_diag_error = ( - "Simulation was not configured with Particle Count info diagnostic" - )Likely invalid or redundant comment.
53-53: Fix unexpected indentation.There is an unexpected indentation at line 53.
- for ilvl, lvl in pcount_hier.levels(time).items(): + for ilvl, lvl in pcount_hier.levels(time).items():Likely invalid or redundant comment.
nicolasaunai
force-pushed
the
refac_hierarchies
branch
from
bd95bb0 to
0c0723e
Compare
- particle count in run - refactor the python hierarchy module - add hierarchy pharesee tests - can set a patchData w. an all_primal centering
nicolasaunai
force-pushed
the
refac_hierarchies
branch
from
0c0723e to
c83a1fc
Compare
There was a problem hiding this comment.
Actionable comments posted: 39
Outside diff range, codebase verification and nitpick comments (1)
pyphare/pyphare/pharesee/run/utils.py (1)
Line range hint
349-368:
Ensure consistent return statements.Avoid mixing explicit and implicit returns for better readability and consistency.
- if ndim == 1: - pass + if ndim == 1: + return NoneTools
Ruff
264-269: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
GitHub Check: CodeQL
[notice] 349-349: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (49)
- pyphare/pyphare/core/box.py (1 hunks)
- pyphare/pyphare/core/operators.py (1 hunks)
- pyphare/pyphare/core/phare_utilities.py (1 hunks)
- pyphare/pyphare/cpp/init.py (1 hunks)
- pyphare/pyphare/pharein/init.py (2 hunks)
- pyphare/pyphare/pharein/diagnostics.py (2 hunks)
- pyphare/pyphare/pharein/examples/job.py (7 hunks)
- pyphare/pyphare/pharein/maxwellian_fluid_model.py (1 hunks)
- pyphare/pyphare/pharein/restarts.py (2 hunks)
- pyphare/pyphare/pharein/simulation.py (5 hunks)
- pyphare/pyphare/pharesee/geometry.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/init.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromh5.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/fromsim.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patch.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchlevel.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/scalarfield.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/vectorfield.py (1 hunks)
- pyphare/pyphare/pharesee/particles.py (1 hunks)
- pyphare/pyphare/pharesee/plotting.py (1 hunks)
- pyphare/pyphare/pharesee/run/init.py (1 hunks)
- pyphare/pyphare/pharesee/run/run.py (1 hunks)
- pyphare/pyphare/pharesee/run/utils.py (4 hunks)
- pyphare/pyphare/simulator/simulator.py (2 hunks)
- pyphare/pyphare_tests/test_pharesee/CMakeLists.txt (1 hunks)
- pyphare/pyphare_tests/test_pharesee/init.py (3 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry.py (2 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (6 hunks)
- pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1 hunks)
- src/amr/tagging/default_hybrid_tagger_strategy.hpp (1 hunks)
- tests/amr/data/field/coarsening/test_coarsen_field.py (1 hunks)
- tests/amr/data/field/refine/test_refine_field.py (1 hunks)
- tests/core/data/gridlayout/test_laplacian.py (1 hunks)
- tests/diagnostic/init.py (1 hunks)
- tests/functional/alfven_wave/alfven_wave1d.py (2 hunks)
- tests/functional/conservation/conserv.py (3 hunks)
- tests/functional/tdtagged/td1dtagged.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 (6 hunks)
- tests/simulator/test_diagnostic_timestamps.py (2 hunks)
- tests/simulator/test_diagnostics.py (1 hunks)
- tests/simulator/test_initialization.py (2 hunks)
- tests/simulator/test_restarts.py (4 hunks)
- tests/simulator/test_run.py (6 hunks)
- tools/python3/phloping.py (1 hunks)
Files skipped from review due to trivial changes (9)
- pyphare/pyphare/core/box.py
- pyphare/pyphare/cpp/init.py
- pyphare/pyphare/pharein/diagnostics.py
- pyphare/pyphare/pharesee/particles.py
- pyphare/pyphare/pharesee/run/init.py
- pyphare/pyphare_tests/test_pharesee/CMakeLists.txt
- tests/amr/data/field/refine/test_refine_field.py
- tests/diagnostic/init.py
- tests/simulator/refinement/test_2d_2_core.py
Files skipped from review as they are similar to previous changes (20)
- pyphare/pyphare/core/phare_utilities.py
- pyphare/pyphare/pharein/init.py
- pyphare/pyphare/pharein/examples/job.py
- pyphare/pyphare/pharein/maxwellian_fluid_model.py
- pyphare/pyphare/pharein/restarts.py
- pyphare/pyphare/pharein/simulation.py
- pyphare/pyphare/pharesee/geometry.py
- pyphare/pyphare/pharesee/hierarchy/init.py
- pyphare/pyphare/pharesee/hierarchy/patchlevel.py
- pyphare/pyphare/pharesee/hierarchy/scalarfield.py
- pyphare/pyphare/simulator/simulator.py
- pyphare/pyphare_tests/test_pharesee/init.py
- src/amr/tagging/default_hybrid_tagger_strategy.hpp
- tests/amr/data/field/coarsening/test_coarsen_field.py
- tests/core/data/gridlayout/test_laplacian.py
- tests/functional/alfven_wave/alfven_wave1d.py
- tests/simulator/refinement/test_2d_10_core.py
- tests/simulator/test_diagnostics.py
- tests/simulator/test_initialization.py
- tests/simulator/test_run.py
Additional context used
GitHub Check: CodeQL
pyphare/pyphare/pharesee/hierarchy/patch.py
[notice] 49-49: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/vectorfield.py
[notice] 85-85: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[notice] 57-57: Unused local variable
Variable self is not used.
[notice] 58-58: Unused local variable
Variable other is not used.
[notice] 80-80: Unused local variable
Variable self is not used.
[notice] 81-81: Unused local variable
Variable other is not used.pyphare/pyphare/pharesee/plotting.py
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.pyphare/pyphare/pharesee/run/run.py
[notice] 260-260: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
[notice] 38-38: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 235-235: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 12-12: Unused import
Import of '_current1d' is not used.pyphare/pyphare/pharesee/hierarchy/fromh5.py
[notice] 8-8: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 9-9: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 17-17: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy_utils begins an import cycle.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
[notice] 193-193: Imprecise assert
assertTrue(a > b) cannot provide an informative message. Using assertGreater(a, b) instead will give more informative messages.
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.pyphare/pyphare_tests/test_pharesee/test_geometry.py
[notice] 295-295: Imprecise assert
assertTrue(a is b) cannot provide an informative message. Using assertIs(a, b) instead will give more informative messages.pyphare/pyphare/pharesee/run/utils.py
[notice] 349-349: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
[notice] 1-1: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.pyphare/pyphare/pharesee/hierarchy/hierarchy.py
[notice] 553-553: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.
[warning] 143-143: Comparison of identical values
Comparison of identical values; use cmath.isnan() if testing for not-a-number.
[failure] 186-186: Potentially uninitialized local variable
Local variable 'slice_dim' may be used before it is initialized.
[failure] 187-187: Potentially uninitialized local variable
Local variable 'cst_dim' may be used before it is initialized.
[notice] 565-565: Cyclic import
Import of module pyphare.pharesee.plotting begins an import cycle.
[notice] 563-563: Module is imported with 'import' and 'import from'
Module 'copy' is imported with both 'import' and 'import from'.
Ruff
tools/python3/phloping.py
43-43: Local variable
bad_diag_erroris assigned to but never usedRemove assignment to unused variable
bad_diag_error(F841)
pyphare/pyphare/pharesee/run/run.py
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
pyphare/pyphare/pharesee/hierarchy/fromh5.py
27-27: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
98-98: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
246-249: Use ternary operator
selection_box = hier.selection_box[time] if hier.selection_box is not None else Noneinstead ofif-else-blockReplace
if-else-block withselection_box = hier.selection_box[time] if hier.selection_box is not None else None(SIM108)
320-320: Do not
assert False(python -Oremoves these calls), raiseAssertionError()Replace
assert False(B011)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
54-54: Ambiguous variable name:
l(E741)
tests/simulator/test_restarts.py
160-160: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
165-165: Import
timefrom line 3 shadowed by loop variable(F402)
pyphare/pyphare/pharesee/run/utils.py
264-269: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
91-91: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
26-26: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
245-245: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
246-246: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
270-270: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
593-593: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
626-626: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
638-638: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
tests/simulator/test_advance.py
362-362: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
Additional comments not posted (81)
pyphare/pyphare/pharesee/hierarchy/patch.py (4)
9-32: Consider removing redundancy in attribute initialization.The initialization has some redundant code that can be simplified.
- if layout is not None: - self.layout = layout - self.box = layout.box - self.origin = layout.origin - self.dl = layout.dl - self.patch_datas = patch_datas - self.id = patch_id - if len(patch_datas): - pdata0 = list(patch_datas.values())[0] # 0 represents all others - self.layout = pdata0.layout - self.box = pdata0.layout.box - self.origin = pdata0.layout.origin - self.dl = pdata0.layout.dl - self.patch_datas = patch_datas - self.id = patch_id + if layout is None and len(patch_datas): + layout = list(patch_datas.values())[0].layout + self.layout = layout + self.box = layout.box + self.origin = layout.origin + self.dl = layout.dl + self.patch_datas = patch_datas + self.id = patch_id
34-38: LGTM!The
__str__and__repr__methods provide clear and consistent string representations of thePatchobject.
40-48: LGTM!The
copyand__copy__methods correctly usedeepcopyto create copies of thePatchobject.
49-66: Ensure consistent return statements.The method mixes explicit and implicit return statements. Refactor to make returns explicit.
- elif idim == 1: + else: return pd.dataset[nbrGhosts:-nbrGhosts, idx + nbrGhosts]Tools
GitHub Check: CodeQL
[notice] 49-49: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/vectorfield.py (5)
15-29: LGTM!The
__init__method correctly initializes theVectorFieldclass.
31-38: LGTM!The
__mul__and__rmul__methods correctly handle multiplication with a scalar.
39-60: Review the__add__method.
- The method handles addition with another
VectorFieldbut raises aRuntimeErrorfor unhandled types.- There is an unused reassignment to
selfandotherwhich should be removed.- self = rename(h_self, names_self_kept) # needed ? - other = rename(h_other, names_other_kept)Tools
GitHub Check: CodeQL
[notice] 57-57: Unused local variable
Variable self is not used.
[notice] 58-58: Unused local variable
Variable other is not used.
62-83: Review the__sub__method.
- The method handles subtraction with another
VectorFieldbut raises aRuntimeErrorfor unhandled types.- There is an unused reassignment to
selfandotherwhich should be removed.- self = rename(h_self, names_self_kept) - other = rename(h_other, names_other_kept)Tools
GitHub Check: CodeQL
[notice] 80-80: Unused local variable
Variable self is not used.
[notice] 81-81: Unused local variable
Variable other is not used.
85-100: Review the__truediv__method.
- The method handles division by a
ScalarFieldor a scalar.- There is a mix of implicit and explicit returns which should be consistent.
- elif isinstance(other, (int, float)): + else:Tools
GitHub Check: CodeQL
[notice] 85-85: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/core/operators.py (8)
8-19: LGTM!The
_compute_dot_productfunction correctly computes the dot product of two vectors.
22-29: LGTM!The
_compute_sqrtfunction correctly computes the square root of a dataset.
32-52: LGTM!The
_compute_cross_productfunction correctly computes the cross product of two vectors.
83-99: LGTM!The
dotfunction correctly computes the dot product of twoVectorFieldobjects.
102-118: LGTM!The
crossfunction correctly computes the cross product of twoVectorFieldobjects.
121-127: LGTM!The
sqrtfunction correctly computes the square root of aScalarFieldobject.
130-133: LGTM!The
modulusfunction correctly computes the modulus of aVectorFieldobject.
136-141: LGTM!The
gradfunction correctly computes the gradient of aScalarFieldobject.tools/python3/phloping.py (3)
49-49: Initialize dictionary usingdefaultdict.Consider using
defaultdictfrom thecollectionsmodule to simplify the dictionary initialization.+ from collections import defaultdict - particles_per_level_per_time_step = {} + particles_per_level_per_time_step = defaultdict(lambda: np.zeros(len(all_times), dtype=int))Likely invalid or redundant comment.
46-46: Concatenate file path usingos.path.join.To ensure compatibility across different operating systems, use
os.path.jointo concatenate file paths.+ import os - filepath = self.run.path + "/particle_count.h5" + filepath = os.path.join(self.run.path, "particle_count.h5")Likely invalid or redundant comment.
41-41: Unused import statement.The import statement for
get_times_from_h5should be moved to the top of the file for better readability and maintainability.- from pyphare.pharesee.hierarchy.fromh5 import get_times_from_h5Likely invalid or redundant comment.
tests/simulator/test_diagnostic_timestamps.py (2)
15-17: LGTM!The reorganization of import statements improves clarity and maintainability.
181-181: LGTM!The update to use
format_timestampdirectly enhances clarity.pyphare/pyphare/pharesee/hierarchy/fromsim.py (2)
57-57: Handle potential KeyError.Ensure that the key
popexists in the dictionary returned bygetters[qty].- populationdict = getters[qty](pop)[pop] + populationdict = getters[qty](pop).get(pop, {})
1-9: Organize imports.The import statements should be organized at the top of the file for better readability and maintainability.
- from .hierarchy_utils import isFieldQty, field_qties, quantidic, refinement_ratio - from .patchdata import FieldData, ParticleData - from .patch import Patch - from .patchlevel import PatchLevel - from .hierarchy import PatchHierarchy - from ..particles import Particles - from ...core.gridlayout import GridLayout - from ...core.box import Box + from .hierarchy_utils import isFieldQty, field_qties, quantidic, refinement_ratio + from .patchdata import FieldData, ParticleData + from .patch import Patch + from .patchlevel import PatchLevel + from .hierarchy import PatchHierarchy + from ..particles import Particles + from ...core.gridlayout import GridLayout + from ...core.box import BoxLikely invalid or redundant comment.
tests/functional/tdtagged/td1dtagged.py (1)
266-266: Update function parameter name to reflect new usage.The parameter
timehas been changed totimes, indicating that the function now handles multiple time values. Ensure that all calls to this function are updated accordingly.Verification successful
Update function parameter name to reflect new usage.
The parameter
timehas been correctly updated totimesin all relevant function calls toget_time.
tests/functional/tdtagged/td1dtagged.pytests/simulator/refinement/test_2d_2_core.pytests/simulator/refinement/test_2d_10_core.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `get_time` match the new parameter `times`. # Test: Search for the function usage. Expect: Only occurrences of the new parameter `times`. rg --type python -A 5 $'get_time'Length of output: 63
Script:
#!/bin/bash # Description: Verify all function calls to `get_time` match the new parameter `times`. # Test: Search for the function usage. Expect: Only occurrences of the new parameter `times`. rg --type py -A 5 $'get_time'Length of output: 12437
Script:
#!/bin/bash # Description: Verify all function calls to `get_time` use the updated parameter `times`. # Test: Search for the function definition and its calls. Expect: Only occurrences of the new parameter `times`. rg --type py -A 5 'def get_time' | rg 'times'Length of output: 936
pyphare/pyphare/pharesee/hierarchy/patchdata.py (7)
8-27: LGTM!The
PatchDataclass is implemented correctly, factoring common geometrical properties forFieldDataandParticleData.
112-112: Fix: Address the attribute overwrite warning.The
layoutattribute is being overwritten, which can lead to unexpected behavior.- def __init__(self, layout, field_name, data, **kwargs): + def __init__(self, layout, field_name, data, **kwargs): + self._layout = layout
37-46: Optimization: Avoid redundant checks in thexproperty.The
withGhostsvariable can be defined once at the class level and reused in all coordinate properties.- @property - def x(self): - withGhosts = self.field_name != "tags" - if self._x is None: - self._x = self.layout.yeeCoordsFor( - self.field_name, - "x", - withGhosts=withGhosts, - centering=self.centerings[0], - ) - return self._x + @property + def x(self): + if self._x is None: + self._x = self.layout.yeeCoordsFor( + self.field_name, + "x", + withGhosts=self._withGhosts, + centering=self.centerings[0], + ) + return self._x
49-58: Optimization: Avoid redundant checks in theyproperty.The
withGhostsvariable can be defined once at the class level and reused in all coordinate properties.- @property - def y(self): - withGhosts = self.field_name != "tags" - if self._y is None: - self._y = self.layout.yeeCoordsFor( - self.field_name, - "y", - withGhosts=withGhosts, - centering=self.centerings[1], - ) - return self._y + @property + def y(self): + if self._y is None: + self._y = self.layout.yeeCoordsFor( + self.field_name, + "y", + withGhosts=self._withGhosts, + centering=self.centerings[1], + ) + return self._y
61-70: Optimization: Avoid redundant checks in thezproperty.The
withGhostsvariable can be defined once at the class level and reused in all coordinate properties.- @property - def z(self): - withGhosts = self.field_name != "tags" - if self._z is None: - self._z = self.layout.yeeCoordsFor( - self.field_name, - "z", - withGhosts=withGhosts, - centering=self.centerings[2], - ) - return self._z + @property + def z(self): + if self._z is None: + self._z = self.layout.yeeCoordsFor( + self.field_name, + "z", + withGhosts=self._withGhosts, + centering=self.centerings[2], + ) + return self._z
216-217: Fix: Changesizeto a property method.The
sizemethod should be a property for consistency with theFieldDataclass.- def size(self): + @property + def size(self):
181-215: LGTM!The
ParticleDataclass is implemented correctly, representing particles in a region.tests/functional/conservation/conserv.py (2)
134-137: Improve readability and maintainability.Consider using a list comprehension to improve readability and maintainability.
- tot = 0.0 - for ilvl, lvl in parts.levels().items(): - if lvlNbr == ilvl: - for patch in lvl.patches: - keys = list(patch.patch_datas.keys()) - pdata = patch.patch_datas[keys[0]] - particles = pdata.dataset - per_patch = fun(particles, **kwargs) - tot += per_patch + tot = sum( + fun(patch.patch_datas[list(patch.patch_datas.keys())[0]].dataset, **kwargs) + for ilvl, lvl in parts.levels().items() if lvlNbr == ilvl + for patch in lvl.patches + )
Line range hint
154-175:
Ensure correct access to levels.The
mag_energyfunction has been updated to uselevels()instead ofpatch_levels. Verify that this change correctly accesses the levels and does not introduce any issues.Verification successful
Verification successful: Correct access to levels confirmed.
The
mag_energyfunction's update to uselevels()instead ofpatch_levelscorrectly accesses the levels without introducing issues. Thelevels()method provides the necessary data structure expected bymag_energy.
levels()method is defined and used appropriately.self.time_hieris correctly initialized and updated to store patch levels.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `levels()` in the `mag_energy` function. # Test: Search for the function usage. Expect: Only occurrences of the new method `levels()`. rg --type python -A 5 $'mag_energy'Length of output: 65
Script:
#!/bin/bash # Description: Verify the usage of `levels()` in the `mag_energy` function. # Test: Search for the function usage. Expect: Only occurrences of the new method `levels()`. rg --type py -A 10 'def mag_energy' | rg 'levels()'Length of output: 135
Script:
#!/bin/bash # Description: Verify the definition and usage of `levels()` and `patch_levels`. # Search for the definition of `levels()` method rg --type py 'def levels' # Search for the definition of `patch_levels` method rg --type py 'def patch_levels'Length of output: 315
Script:
#!/bin/bash # Description: Extract definitions and relevant context of `levels()` and `patch_levels_from_h5()` for comparison. # Extract the definition and relevant context of `levels()` method rg --type py -A 10 'def levels' pyphare/pyphare/pharesee/hierarchy.py rg --type py -A 10 'def levels' pyphare/pyphare/pharesee/hierarchy/hierarchy.py # Extract the definition and relevant context of `patch_levels_from_h5()` method rg --type py -A 10 'def patch_levels_from_h5' pyphare/pyphare/pharesee/hierarchy/fromh5.pyLength of output: 1272
Script:
#!/bin/bash # Description: Verify the initialization and structure of `self.time_hier`. # Search for `self.time_hier` initialization and usage rg --type py 'self.time_hier'Length of output: 1713
Tools
Ruff
157-157: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
pyphare/pyphare/pharesee/plotting.py (1)
248-248: LGTM!The removal of
gridlayoutimport is valid as it is not used within the file.Tools
GitHub Check: CodeQL
[notice] 248-248: Cyclic import
Import of module pyphare.pharesee.hierarchy.fromh5 begins an import cycle.pyphare/pyphare/pharesee/run/run.py (2)
47-51: Simplify nested if statements.Use a single if statement instead of nested if statements.
Tools
Ruff
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
235-235: Remove redundant import.Module
globis imported with bothimportandimport from. Consolidate the imports to avoid redundancy.Tools
GitHub Check: CodeQL
[notice] 235-235: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.pyphare/pyphare/pharesee/hierarchy/fromh5.py (2)
1-9: Cyclic Import DetectedThe import statements are necessary but introduce cyclic import issues.
Consider refactoring to avoid cyclic imports. One approach could be moving common functions to a separate utility module.
Tools
GitHub Check: CodeQL
[notice] 8-8: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
[notice] 9-9: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
27-27: Avoid using mutable data structures as default arguments.Replace with
Noneand initialize within the function to avoid unexpected behavior.Tools
Ruff
27-27: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (13)
158-161: LGTM!The code changes are approved.
163-173: LGTM!The code changes are approved.
174-182: LGTM!The code changes are approved.
183-187: LGTM!The code changes are approved.
195-201: LGTM!The code changes are approved.
232-238: LGTM!The code changes are approved.
240-247: LGTM!The code changes are approved.
249-257: LGTM!The code changes are approved.
259-269: LGTM!The code changes are approved.
271-280: LGTM!The code changes are approved.
282-293: LGTM!The code changes are approved.
295-300: LGTM!The code changes are approved.
229-230: UseassertLessfor better error messages.The assert statement can be improved for better error messages.
- self.assertTrue(Bpartial.nbrPatches() < B.nbrPatches()) + self.assertLess(Bpartial.nbrPatches(), B.nbrPatches())Likely invalid or redundant comment.
pyphare/pyphare/pharesee/run/utils.py (11)
128-140: LGTM!The function
_ppp_to_ppp_domain_slicingcorrectly implements the slicing logic for (primal, primal, primal) to (primal, primal, primal) centering.
143-161: LGTM!The function
_pdd_to_ppp_domain_slicingcorrectly implements the slicing logic for (dual, primal, primal) to (primal, primal, primal) centering.
164-182: LGTM!The function
_dpd_to_ppp_domain_slicingcorrectly implements the slicing logic for (dual, primal, dual) to (primal, primal, primal) centering.
185-208: LGTM!The function
_ddp_to_ppp_domain_slicingcorrectly implements the slicing logic for (dual, dual, primal) to (primal, primal, primal) centering.
211-229: LGTM!The function
_dpp_to_ppp_domain_slicingcorrectly implements the slicing logic for (dual, primal, primal) to (primal, primal, primal) centering.
232-250: LGTM!The function
_pdp_to_ppp_domain_slicingcorrectly implements the slicing logic for (primal, dual, primal) to (primal, primal, primal) centering.
253-271: LGTM!The function
_ppd_to_ppp_domain_slicingcorrectly implements the slicing logic for (primal, primal, dual) to (primal, primal, primal) centering.Tools
Ruff
264-269: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
285-288: LGTM!The function
merge_centeringscorrectly combines centering configurations using theyee_centeringdictionary and thedirectionslist.
291-292: LGTM!The function
slices_to_primalcorrectly maps merged centerings to the corresponding slicing functions using theslices_to_primal_dictionary.
295-338: LGTM!The function
_compute_to_primalcorrectly processes patch data and handles NaN values in ghost cells.
341-346: LGTM!The function
_inner_slicescorrectly returns slices for inner, inner_shift_left, and inner_shift_right.pyphare/pyphare/pharesee/hierarchy/hierarchy.py (6)
210-213: LGTM!The method is correctly implemented.
228-237: LGTM!The method is correctly implemented.
399-453: LGTM!The method is correctly implemented.
453-551: LGTM!The method is correctly implemented.
Tools
Ruff
487-487: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
675-742: LGTM!The function is correctly implemented.
284-285: Rename unused loop variable.Rename the unused loop variable
ilvlto_ilvlfor better readability.- for _, lvl in self.levels(time).items(): + for _ilvl, lvl in self.levels(time).items():Likely invalid or redundant comment.
tests/simulator/test_advance.py (5)
20-22: LGTM!The changes in the import statements are correctly implemented.
460-460: LGTM!The changes in the function call are correctly implemented.
476-476: LGTM!The changes in the function call are correctly implemented.
562-562: LGTM!The changes in the function call are correctly implemented.
716-716: LGTM!The changes in the random integer generation are correctly implemented.
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (5)
35-46: Simplify compatibility checks usingall().The compatibility checks can be simplified using the
all()function for better readability.def are_compatible_hierarchies(hierarchies): ref = hierarchies[0] same_box = all(hier.domain_box == ref.domain_box for hier in hierarchies[1:]) same_selection = all(hier.selection_box == ref.selection_box for hier in hierarchies[1:]) same_files = all(hier.data_files.keys() == ref.data_files.keys() for hier in hierarchies[1:]) same_times = all(hier.time_hier.keys() == ref.time_hier.keys() for hier in hierarchies[1:]) return same_box and same_selection and same_files and same_times
46-46: Ensure meaningful return value.The function currently always returns
True, which may not be the intended behavior. Consider returning the combined result of the compatibility checks.- return True + return same_box and same_selection and same_files and same_times
49-52: Rename unused loop variables.Rename unused loop variables to
_time,_ilvl, and_ipto clarify their purpose.- for time, patch_levels in hierarchy.time_hier.items(): - for ilvl, plvl in patch_levels.items(): - for ip, patch in enumerate(plvl.patches): + for _time, patch_levels in hierarchy.time_hier.items(): + for _ilvl, plvl in patch_levels.items(): + for _ip, patch in enumerate(plvl.patches):Tools
Ruff
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
91-91: Simplify dictionary key check.Use
key in dictinstead ofkey in dict.keys().- counts = {ilvl: 0 for ilvl in hier.levels().keys()} + counts = {ilvl: 0 for ilvl in hier.levels()}Tools
Ruff
91-91: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
104-104: Improve error message.Provide a more descriptive error message for better debugging.
- raise RuntimeError("more than one patch found for point") + raise RuntimeError(f"More than one patch found for point {point} at level {k}")pyphare/pyphare_tests/test_pharesee/test_geometry_2d.py (2)
117-117: Rename unused loop variable.Rename unused loop variable to
_lvlto clarify its purpose.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
117-117: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
268-268: Rename unused loop variable.Rename unused loop variable to
_lvlto clarify its purpose.- for ilvl, lvl in enumerate(hierarchy.levels().items()): + for ilvl, _lvl in enumerate(hierarchy.levels().items()):Tools
Ruff
268-268: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
PhilipDeegan
left a comment
PhilipDeegan
left a comment
There was a problem hiding this comment.
some final remarks before merge
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patch.py (1 hunks)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py (1 hunks)
- tests/simulator/test_load_balancing.py (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/simulator/test_load_balancing.py
Files skipped from review as they are similar to previous changes (1)
- pyphare/pyphare/pharesee/hierarchy/patchdata.py
Additional context used
GitHub Check: CodeQL
pyphare/pyphare/pharesee/hierarchy/patch.py
[notice] 52-52: Explicit returns mixed with implicit (fall through) returns
Mixing implicit and explicit returns may indicate an error as implicit returns always return None.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
[notice] 1-1: Cyclic import
Import of module pyphare.pharesee.hierarchy.hierarchy begins an import cycle.
Ruff
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
92-92: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
584-584: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
597-597: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
Additional comments not posted (16)
pyphare/pyphare/pharesee/hierarchy/patch.py (5)
34-35: LGTM!The
__str__method is simple and correct.
37-38: LGTM!The
__repr__method is simple and correct.
40-41: LGTM!The
__getitem__method is simple and correct.
43-47: LGTM!The
copymethod is simple and correct.
49-50: LGTM!The
__copy__method is simple and correct.pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py (11)
170-171: LGTM!The
is_root_lvlfunction is simple and correct.
195-216: LGTM!The
isFieldQtyfunction is simple and correct.
301-321: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that
254-298: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly returns the mask for overlapping patch data.
Verification successful
Let's correct the file type and re-run the script to search for the usage of the
overlap_mask_2dfunction.
LGTM! But verify performance and correctness.
The code changes are approved.
The
overlap_mask_2dfunction is used within the same file to filter data based on the overlap mask, indicating its correctness in identifying overlapping regions. Ensure that the function performs well in practice.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `overlap_mask_2d` function. # Test: Search for the function usage. Expect: Correct return of the mask for overlapping patch data. rg --type python -A 5 $'overlap_mask_2d'Length of output: 70
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `overlap_mask_2d` function. # Test: Search for the function usage. Expect: Correct return of the mask for overlapping patch data. rg --type py -A 5 $'overlap_mask_2d'Length of output: 1179
219-251: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly returns the mask for overlapping patch data.
Verification successful
The
overlap_mask_1dfunction is correctly implemented and used within the codebase.The function is used to create a mask for overlapping patch data, which is then utilized to filter and concatenate data arrays. This matches the function's intended purpose.
- File:
pyphare/pyphare/pharesee/hierarchy/hierarchy_utils.py- Lines: 219-251
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `overlap_mask_1d` function. # Test: Search for the function usage. Expect: Correct return of the mask for overlapping patch data. rg --type python -A 5 $'overlap_mask_1d'Length of output: 70
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `overlap_mask_1d` function. # Test: Search for the function usage. Expect: Correct return of the mask for overlapping patch data. rg --type py -A 5 $'overlap_mask_1d'Length of output: 1163
Script:
#!/bin/bash # Description: Extract more lines around the usage of the `overlap_mask_1d` function to understand its context and verify correctness. # Extract 10 lines before and after the usage of `overlap_mask_1d` rg --type py -C 10 $'overlap_mask_1d'Length of output: 3698
135-143: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly extracts patch data.
109-132: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly creates a new hierarchy.
Verification successful
The
compute_hier_fromfunction is correctly utilized and the callback functions are properly defined.The function is used extensively across the codebase, and the callback functions handle various computations on patch data effectively.
pyphare/pyphare/pharesee/run/run.pypyphare/pyphare/pharesee/hierarchy/vectorfield.pypyphare/pyphare/pharesee/hierarchy/scalarfield.pypyphare/pyphare/core/operators.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `compute_hier_from` function. # Test: Search for the function usage. Expect: Correct creation of a new hierarchy. rg --type python -A 5 $'compute_hier_from'Length of output: 72
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `compute_hier_from` function. # Test: Search for the function usage. Expect: Correct creation of a new hierarchy. rg --type py -A 5 $'compute_hier_from'Length of output: 15270
Script:
#!/bin/bash # Description: Extract the definitions of the callback functions used with `compute_hier_from`. # Search for the definitions of callback functions used with `compute_hier_from`. rg --type py -A 5 '_compute_to_primal|_compute_pop_pressure|_compute_pressure|_compute_current|_compute_divB|_get_rank|compute_rename|_compute_mul|_compute_add|_compute_sub|_compute_truediv|_compute_scalardiv|_compute_neg|_compute_dot_product|_compute_cross_product|_compute_sqrt|_compute_grad'Length of output: 24759
49-83: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly merges particle data.
Verification successful
The
merge_particlesfunction is adequately tested for performance and correctness.The test cases in
tests/simulator/test_initialization.pyandtests/simulator/test_advance.pyintegratemerge_particlesinto the simulation processes, ensuring that particle data is correctly merged.
tests/simulator/test_initialization.pytests/simulator/test_advance.pyScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `merge_particles` function. # Test: Search for the function usage. Expect: Correct merging of particle data. rg --type python -A 5 $'merge_particles'Length of output: 70
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `merge_particles` function. # Correct the command to search for the function usage in Python files. rg -t py -A 5 'merge_particles' # Additionally, search for the function definition to understand its context. rg -t py 'def merge_particles'Length of output: 2389
Script:
#!/bin/bash # Description: Extract relevant test cases for the `merge_particles` function. # Extract test cases from the test files where `merge_particles` is used. rg -A 10 'merge_particles' tests/simulator/test_initialization.py rg -A 10 'merge_particles' tests/simulator/test_advance.pyLength of output: 1589
Script:
#!/bin/bash # Description: Extract more lines around the test cases for the `merge_particles` function. # Extract additional lines from the test files where `merge_particles` is used to understand the test cases better. rg -A 20 'merge_particles' tests/simulator/test_initialization.py rg -A 20 'merge_particles' tests/simulator/test_advance.pyLength of output: 2933
Tools
Ruff
50-50: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
51-51: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
52-52: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
146-152: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly creates new patch data.
Verification successful
The
new_patchdatas_fromfunction appears to be covered indirectly by existing tests for related components.
- Tests for
hierarchy_utils.pyandFieldDataare present, suggesting that the functionality ofnew_patchdatas_fromis being tested.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `new_patchdatas_from` function. # Test: Search for the function usage. Expect: Correct creation of new patch data. rg --type python -A 5 $'new_patchdatas_from'Length of output: 74
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `new_patchdatas_from` function. # Test: Search for the function usage. Expect: Correct creation of new patch data. rg --type py -A 5 $'new_patchdatas_from'Length of output: 1214
Script:
#!/bin/bash # Description: Verify the presence of tests for the `new_patchdatas_from` function. # Test: Search for test cases. Expect: Presence of test cases for `new_patchdatas_from`. rg --type py -A 5 $'new_patchdatas_from' | grep -i 'test'Length of output: 54
Script:
#!/bin/bash # Description: Verify the presence of tests for the `new_patchdatas_from` function. # Test: Search for test cases in test files. Expect: Presence of test cases for `new_patchdatas_from`. rg 'new_patchdatas_from' --type py --glob '*test*.py' -A 5Length of output: 58
Script:
#!/bin/bash # Description: Verify the presence of tests for components related to `new_patchdatas_from` function. # Test: Search for test cases related to `hierarchy_utils.py` and `FieldData`. Expect: Presence of test cases for related components. rg 'hierarchy_utils' --type py --glob '*test*.py' -A 5 rg 'FieldData' --type py --glob '*test*.py' -A 5Length of output: 7645
174-192: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly returns the dictionary of quantities.
Verification successful
LGTM! But verify performance and correctness.
The code changes are approved.
However, ensure that the function performs well and correctly returns the dictionary of quantities.
LGTM! But verify performance and correctness.
The code changes are approved.
However, ensure that the function performs well and correctly returns the dictionary of quantities.
LGTM! But verify performance and correctness.
The code changes are approved.
However, ensure that the function performs well and correctly returns the dictionary of quantities.
LGTM! But verify performance and correctness.
The code changes are approved.
However, ensure that the function performs well and correctly returns the dictionary of quantities.
- The
quantidicfunction is used inpyphare/pyphare/pharesee/hierarchy/fromsim.pyto populate thegettersdictionary.- The dictionary values are accessed by key to retrieve patches, indicating correct usage.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `quantidic` function. # Test: Search for the function usage. Expect: Correct return of the dictionary of quantities. rg --type python -A 5 $'quantidic'Length of output: 64
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `quantidic` function. # Test: Search for the function usage. Expect: Correct return of the dictionary of quantities. rg --type py -A 5 $'quantidic'Length of output: 1499
Script:
#!/bin/bash # Description: Verify the correctness of the `quantidic` function usage in `fromsim.py`. # Test: Extract the relevant lines of code where `quantidic` is used in `fromsim.py`. rg --type py -A 10 $'quantidic(ilvl, dw)' pyphare/pyphare/pharesee/hierarchy/fromsim.pyLength of output: 88
Script:
#!/bin/bash # Description: Verify the correctness of the `quantidic` function usage in `fromsim.py`. # Test: Extract the relevant lines of code where `quantidic` is used in `fromsim.py`. rg --type py -A 10 'quantidic' pyphare/pyphare/pharesee/hierarchy/fromsim.pyLength of output: 790
155-167: LGTM! But verify performance and correctness.The code changes are approved.
However, ensure that the function performs well and correctly creates new patches.
Verification successful
LGTM! The
new_patches_fromfunction is adequately tested through the existingPatchHierarchytests.The code changes are approved, and the function's performance and correctness are verified through indirect testing.
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py: Contains tests that validate the functionality ofPatchHierarchy, indirectly ensuring the correctness ofnew_patches_from.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `new_patches_from` function. # Test: Search for the function usage. Expect: Correct creation of new patches. rg --type python -A 5 $'new_patches_from'Length of output: 71
Script:
#!/bin/bash # Description: Verify the performance and correctness of the `new_patches_from` function. # Test: Search for the function usage. Expect: Correct creation of new patches. rg --type py -A 5 $'new_patches_from'Length of output: 1197
Script:
#!/bin/bash # Description: Gather more context around the usage of the `new_patches_from` function and check for related tests. # Extract more lines around the usage of `new_patches_from` for better context. rg --type py -A 20 -B 10 $'new_patches_from' # Search for any test files that might be testing `new_patches_from`. rg --type py 'new_patches_from' --files-with-matches | grep 'test'Length of output: 5645
Script:
#!/bin/bash # Description: Search for tests that might indirectly test the `new_patches_from` function by looking for tests related to `PatchHierarchy`. # Search for test files that mention `PatchHierarchy`. rg --type py 'PatchHierarchy' --files-with-matches | grep 'test'Length of output: 164
Script:
#!/bin/bash # Description: Extract the contents of test files that mention `PatchHierarchy` to verify if `new_patches_from` is adequately tested. # Display the contents of the test files. cat pyphare/pyphare_tests/test_pharesee/__init__.py cat pyphare/pyphare_tests/test_pharesee/test_hierarchy.pyLength of output: 19297
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)
- tests/simulator/test_load_balancing.py (2 hunks)
Files skipped from review due to trivial changes (1)
- tests/simulator/test_load_balancing.py
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)
- tests/simulator/test_load_balancing.py (5 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/simulator/test_load_balancing.py
There was a problem hiding this comment.
Actionable comments posted: 12
Outside diff range, codebase verification and nitpick comments (1)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1)
1-6: Group imports from the same module.Grouping imports from the same module can improve readability.
- from ...core.box import Box - from ...core import box as boxm - from ...core.phare_utilities import refinement_ratio - from ...core.phare_utilities import listify + from ...core import Box, box as boxm, refinement_ratio, listify
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- pyphare/pyphare/pharesee/hierarchy/hierarchy.py (1 hunks)
Additional context used
Learnings (1)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (2)
Learnt from: PhilipDeegan PR: PHAREHUB/PHARE#859 File: pyphare/pyphare/pharesee/hierarchy/hierarchy.py:26-26 Timestamp: 2024-07-23T09:51:46.471Z Learning: Mutable default arguments can lead to unexpected behavior because the default value is shared between all calls to the function. Replacing the mutable default with `None` and initializing it within the function ensures that a new list is created each time the function is called, avoiding this issue.Learnt from: PhilipDeegan PR: PHAREHUB/PHARE#859 File: pyphare/pyphare/pharesee/hierarchy/fromh5.py:27-34 Timestamp: 2024-07-23T09:51:46.761Z Learning: Mutable default arguments like lists or dictionaries can lead to unexpected behavior because the same instance of the mutable object is shared across all calls to the function. It is a good practice to use `None` as the default value and initialize the mutable object inside the function.
Ruff
pyphare/pyphare/pharesee/hierarchy/hierarchy.py
26-26: Do not use mutable data structures for argument defaults
Replace with
None; initialize within function(B006)
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
130-130: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling(B904)
244-244: Loop control variable
timenot used within loop bodyRename unused
timeto_time(B007)
245-245: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
269-269: Loop control variable
ilvlnot used within loop bodyRename unused
ilvlto_ilvl(B007)
486-486: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
592-592: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
625-625: Use
key in dictinstead ofkey in dict.keys()Remove
.keys()(SIM118)
637-637: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
Additional comments not posted (10)
pyphare/pyphare/pharesee/hierarchy/hierarchy.py (10)
12-15: LGTM!The
format_timestampfunction is simple and correctly handles the formatting.
71-72: LGTM!The
__getitem__method is simple and correctly retrieves an item from the dictionary.
95-102: LGTM!The
nbytesmethod is simple and correctly calculates the total number of bytes.
104-109: LGTM!The
nbrPatchesmethod is simple and correctly calculates the total number of patches.
209-212: LGTM!The
levelsmethod is simple and correctly retrieves levels based on time.
398-451: LGTM!The
plot1dfunction correctly handles plotting of 1D data.
452-549: LGTM!The
plot2dfunction correctly handles plotting of 2D data.Tools
Ruff
486-486: Loop control variable
lvlnot used within loop bodyRename unused
lvlto_lvl(B007)
558-609: LGTM!The
dist_plotfunction correctly handles plotting of phase space of a particle hierarchy.Tools
Ruff
592-592: Loop control variable
ipnot used within loop bodyRename unused
ipto_ip(B007)
350-353: **LGTM!
283-284: Rename unused loop variables.Rename the unused loop variable
ilvlto_ilvlfor better readability.- for _, lvl in self.levels(time).items(): + for _ilvl, lvl in self.levels(time).items():Likely invalid or redundant comment.
| # but the "serialized_simulation" in "py_attrs" should be the same for all files | ||
| # used by the hierarchy. So we just take the first one. | ||
| first_file = list(self.data_files.values())[0] | ||
| if "py_attrs" not in first_file.keys(): |
There was a problem hiding this comment.
Simplify dictionary key check.
Use key not in dict instead of key not in dict.keys() for better readability and performance.
- if "py_attrs" not in first_file.keys():
+ if "py_attrs" not in first_file: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.
| if "py_attrs" not in first_file.keys(): | |
| if "py_attrs" not in first_file: |
Tools
Ruff
120-120: Use
key not in dictinstead ofkey not in dict.keys()Remove
.keys()(SIM118)
There was a problem hiding this comment.
Actionable comments posted: 11
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- pyphare/pyphare/core/operators.py (1 hunks)
- pyphare/pyphare/pharesee/run/run.py (1 hunks)
- pyphare/pyphare_tests/test_pharesee/test_geometry.py (2 hunks)
- pyphare/pyphare_tests/test_pharesee/test_hierarchy.py (1 hunks)
- tests/simulator/test_advance.py (6 hunks)
- tools/python3/phloping.py (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- pyphare/pyphare/core/operators.py
- pyphare/pyphare_tests/test_pharesee/test_geometry.py
- tests/simulator/test_advance.py
Additional context used
Ruff
pyphare/pyphare/pharesee/run/run.py
12-12:
.utils._current1dimported but unusedRemove unused import:
.utils._current1d(F401)
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
235-235:
globimported but unusedRemove unused import:
glob(F401)
259-259:
glob.globimported but unusedRemove unused import:
glob.glob(F401)
263-263: Local variable
pathis assigned to but never usedRemove assignment to unused variable
path(F841)
pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
54-54: Ambiguous variable name:
l(E741)
GitHub Check: CodeQL
pyphare/pyphare/pharesee/run/run.py
[notice] 38-38: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 12-12: Unused import
Import of '_current1d' is not used.
[notice] 235-235: Module is imported with 'import' and 'import from'
Module 'glob' is imported with both 'import' and 'import from'.
[notice] 260-260: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
[notice] 263-263: Unused local variable
Variable path is not used.pyphare/pyphare_tests/test_pharesee/test_hierarchy.py
[notice] 26-26: Module is imported with 'import' and 'import from'
Module 'pyphare.pharein' is imported with both 'import' and 'import from'.
Additional comments not posted (2)
tools/python3/phloping.py (1)
52-53: Optimize level tracking.Instead of appending to
seen_levels, consider using a set for efficient level tracking.- if ilvl not in seen_levels: - seen_levels += [ilvl] + seen_levels.add(ilvl)Likely invalid or redundant comment.
pyphare/pyphare/pharesee/run/run.py (1)
55-55: Concatenate file path usingos.path.join.To ensure compatibility across different operating systems, use
os.path.jointo concatenate file paths.- h5_filename=os.path.join(self.path, filename),Likely invalid or redundant comment.
| from pyphare.pharesee.hierarchy.fromh5 import get_times_from_h5 | ||
|
|
There was a problem hiding this comment.
Import get_times_from_h5 at the top of the file.
It is a good practice to import all modules and functions at the top of the file to improve readability and maintainability.
- from pyphare.pharesee.hierarchy.fromh5 import get_times_from_h5Move the import statement to the top of the file.
from pyphare.pharesee.hierarchy.fromh5 import get_times_from_h5| filepath = self.run.path + "/particle_count.h5" | ||
| all_times = get_times_from_h5(filepath) |
There was a problem hiding this comment.
Concatenate file path using os.path.join.
To ensure compatibility across different operating systems, use os.path.join to concatenate file paths.
- filepath = self.run.path + "/particle_count.h5"
+ import os
+ filepath = os.path.join(self.run.path, "particle_count.h5")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.
| filepath = self.run.path + "/particle_count.h5" | |
| all_times = get_times_from_h5(filepath) | |
| import os | |
| filepath = os.path.join(self.run.path, "particle_count.h5") | |
| all_times = get_times_from_h5(filepath) |
| filepath = self.run.path + "/particle_count.h5" | ||
| all_times = get_times_from_h5(filepath) | ||
|
|
||
| particles_per_level_per_time_step = {} |
There was a problem hiding this comment.
Initialize dictionary using defaultdict.
Consider using defaultdict from the collections module to simplify the dictionary initialization.
+ from collections import defaultdict
- particles_per_level_per_time_step = {}
+ particles_per_level_per_time_step = defaultdict(lambda: np.zeros(len(all_times), dtype=int))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.
| particles_per_level_per_time_step = {} | |
| from collections import defaultdict | |
| particles_per_level_per_time_step = defaultdict(lambda: np.zeros(len(all_times), dtype=int)) |
| if ilvl not in particles_per_level_per_time_step: | ||
| particles_per_level_per_time_step[ilvl] = np.zeros( | ||
| len(all_times), dtype=int | ||
| ) |
There was a problem hiding this comment.
Simplify dictionary initialization.
Using defaultdict simplifies the dictionary initialization and avoids the need for explicit checks.
- if ilvl not in particles_per_level_per_time_step:
- particles_per_level_per_time_step[ilvl] = np.zeros(
- len(all_times), dtype=int
- )Committable suggestion was skipped due to low confidence.
| import os | ||
| import h5py | ||
|
|
||
| path = self.path |
There was a problem hiding this comment.
Remove unused variable.
The variable path is assigned but never used. It should be removed to clean up the code.
- path = self.pathCommittable 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.
| path = self.path |
Tools
Ruff
263-263: Local variable
pathis assigned to but never usedRemove assignment to unused variable
path(F841)
GitHub Check: CodeQL
[notice] 263-263: Unused local variable
Variable path is not used.
|
|
||
| def all_times(self): | ||
| from glob import glob | ||
| import os |
There was a problem hiding this comment.
Remove redundant import.
The import of module os is redundant, as it was previously imported on line 1.
- import osCommittable 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.
| import os |
Tools
GitHub Check: CodeQL
[notice] 260-260: Module is imported more than once
This import of module os is redundant, as it was previously imported on line 1.
| # assumes all qties in the hierarchy have the same ghost width | ||
| # so take the first patch data of the first patch of the first level.... | ||
| nbrGhosts = list(hierarchy.level(0).patches[0].patch_datas.values())[ | ||
| 0 | ||
| ].ghosts_nbr |
There was a problem hiding this comment.
Simplify redundant checks.
The nested list access can be simplified for better readability.
- # assumes all qties in the hierarchy have the same ghost width
- # so take the first patch data of the first patch of the first level....
- nbrGhosts = list(hierarchy.level(0).patches[0].patch_datas.values())[0].ghosts_nbr
+ # assumes all quantities in the hierarchy have the same ghost width
+ # so take the first patch data of the first patch of the first level....
+ first_patch_data = next(iter(hierarchy.level(0).patches[0].patch_datas.values()))
+ nbrGhosts = first_patch_data.ghosts_nbrCommittable 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.
| # assumes all qties in the hierarchy have the same ghost width | |
| # so take the first patch data of the first patch of the first level.... | |
| nbrGhosts = list(hierarchy.level(0).patches[0].patch_datas.values())[ | |
| 0 | |
| ].ghosts_nbr | |
| # assumes all quantities in the hierarchy have the same ghost width | |
| # so take the first patch data of the first patch of the first level.... | |
| first_patch_data = next(iter(hierarchy.level(0).patches[0].patch_datas.values())) | |
| nbrGhosts = first_patch_data.ghosts_nbr |
| if "selection_box" in kwargs: | ||
| if isinstance(kwargs["selection_box"], tuple): | ||
| lower = kwargs["selection_box"][:2] | ||
| upper = kwargs["selection_box"][2:] | ||
| kwargs["selection_box"] = Box(lower, upper) |
There was a problem hiding this comment.
Combine nested if statements.
Use a single if statement instead of nested if statements.
- if "selection_box" in kwargs:
- if isinstance(kwargs["selection_box"], tuple):
- lower = kwargs["selection_box"][:2]
- upper = kwargs["selection_box"][2:]
- kwargs["selection_box"] = Box(lower, upper)
+ if "selection_box" in kwargs and isinstance(kwargs["selection_box"], tuple):
+ lower = kwargs["selection_box"][:2]
+ upper = kwargs["selection_box"][2:]
+ kwargs["selection_box"] = Box(lower, upper)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.
| if "selection_box" in kwargs: | |
| if isinstance(kwargs["selection_box"], tuple): | |
| lower = kwargs["selection_box"][:2] | |
| upper = kwargs["selection_box"][2:] | |
| kwargs["selection_box"] = Box(lower, upper) | |
| if "selection_box" in kwargs and isinstance(kwargs["selection_box"], tuple): | |
| lower = kwargs["selection_box"][:2] | |
| upper = kwargs["selection_box"][2:] | |
| kwargs["selection_box"] = Box(lower, upper) |
Tools
Ruff
47-48: Use a single
ifstatement instead of nestedifstatements(SIM102)
| self.assertEqual(len(B.levels()), B.levelNbr()) | ||
| self.assertEqual(len(B.levels()), 2) | ||
| self.assertEqual(len(B.levels(time)), 2) | ||
| self.assertEqual(len(B.levels(time)), B.levelNbr(time)) |
There was a problem hiding this comment.
Combine assertions for better readability.
Combining related assertions can improve readability and maintainability.
- self.assertEqual(len(B.levels()), B.levelNbr())
- self.assertEqual(len(B.levels()), 2)
- self.assertEqual(len(B.levels(time)), 2)
- self.assertEqual(len(B.levels(time)), B.levelNbr(time))
+ self.assertEqual(len(B.levels()), B.levelNbr())
+ self.assertEqual(len(B.levels()), 2)
+ self.assertEqual(len(B.levels(time)), 2)
+ self.assertEqual(len(B.levels(time)), B.levelNbr(time))Committable suggestion was skipped due to low confidence.
| def test_data_is_a_hierarchy(self): | ||
| r = Run(diag_outputs) | ||
| B = r.GetB(0.0) | ||
| self.assertTrue(isinstance(B, PatchHierarchy)) |
There was a problem hiding this comment.
Use assertIsInstance for better readability.
The assertIsInstance method provides a more readable and informative assertion.
- self.assertTrue(isinstance(B, PatchHierarchy))
+ self.assertIsInstance(B, PatchHierarchy)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 test_data_is_a_hierarchy(self): | |
| r = Run(diag_outputs) | |
| B = r.GetB(0.0) | |
| self.assertTrue(isinstance(B, PatchHierarchy)) | |
| def test_data_is_a_hierarchy(self): | |
| r = Run(diag_outputs) | |
| B = r.GetB(0.0) | |
| self.assertIsInstance(B, PatchHierarchy) |
3 participants
run particle count
refactoring hierarchy.py in separate files
partial Patch Hierarchies with selection box
refactoring of hierarchy_fromh5 into separate functions
run.Getters can be given a list of times
ScarlarField, VectorField and operators
Run convenience methods for getting times
hierarchy.patch_levels not strictly required anymore
PatchHierarchy unit tests
Also some refractory to split hierarchy.py in different files.
Summary by CodeRabbit
Refactor
New Features
PatchHierarchyclass for efficient multi-dimensional data management and visualization.Bug Fixes