Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug #913

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

bug #913

Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions tests/simulator/refinement/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ endif()

if(HighFive)
## These test use dump diagnostics so require HighFive!
phare_python3_exec(9 harris2d harris.py ${CMAKE_CURRENT_BINARY_DIR})
if(testMPI)
phare_mpi_python3_exec(9 2 simple_2d_refinement test_2d_2_core.py ${CMAKE_CURRENT_BINARY_DIR})
phare_mpi_python3_exec(11 10 complex_2d_refinement test_2d_10_core.py ${CMAKE_CURRENT_BINARY_DIR})
endif(testMPI)

endif()
225 changes: 225 additions & 0 deletions tests/simulator/refinement/harris.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
#!/usr/bin/env python3

import pyphare.pharein as ph # lgtm [py/import-and-import-from]
from pyphare.pharein import Simulation
from pyphare.pharein import MaxwellianFluidModel
from pyphare.pharein import (
ElectromagDiagnostics,
FluidDiagnostics,
ParticleDiagnostics,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused import ParticleDiagnostics.

According to the static analysis hints, ParticleDiagnostics is imported but not used in the code.

Apply this diff to remove the unused import:

     ElectromagDiagnostics,
     FluidDiagnostics,
-    ParticleDiagnostics,
     InfoDiagnostics,
📝 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.

Suggested change
ParticleDiagnostics,
ElectromagDiagnostics,
FluidDiagnostics,
InfoDiagnostics,
🧰 Tools
🪛 Ruff

9-9: pyphare.pharein.ParticleDiagnostics imported but unused

Remove unused import: pyphare.pharein.ParticleDiagnostics

(F401)

InfoDiagnostics,
)
from pyphare.pharein import MetaDiagnostics
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused import MetaDiagnostics.

MetaDiagnostics is imported but not used in the code.

Apply this diff to remove the unused import:

-from pyphare.pharein import MetaDiagnostics
📝 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.

Suggested change
from pyphare.pharein import MetaDiagnostics
🧰 Tools
🪛 Ruff

12-12: pyphare.pharein.MetaDiagnostics imported but unused

Remove unused import: pyphare.pharein.MetaDiagnostics

(F401)

from pyphare.pharein import ElectronModel
from pyphare.simulator.simulator import Simulator, startMPI
from pyphare.pharein import global_vars as gv
from pyphare.pharein import LoadBalancer
import sys
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused import sys.

The sys module is imported but not utilized in the script.

Apply this diff to remove the unused import:

-import sys
📝 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.

Suggested change
import sys
🧰 Tools
🪛 Ruff

17-17: sys imported but unused

Remove unused import: sys

(F401)

import numpy as np
import matplotlib.pyplot as plt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove unused import matplotlib.pyplot as plt.

matplotlib.pyplot is imported as plt but is not used in the code.

Apply this diff to remove the unused import:

-import matplotlib.pyplot as plt
📝 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.

Suggested change
import matplotlib.pyplot as plt
🧰 Tools
🪛 Ruff

19-19: matplotlib.pyplot imported but unused

Remove unused import: matplotlib.pyplot

(F401)

import matplotlib as mpl
from tests.simulator.test_advance import AdvanceTestBase

mpl.use("Agg")
from pyphare.cpp import cpp_lib

cpp = cpp_lib()
startMPI()
Comment on lines +1 to +27
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Clean up imports and improve consistency

There are several unused imports in the file. Consider removing them to improve code clarity:

  1. Remove the unused import of ParticleDiagnostics (line 9).
  2. Remove the unused import of MetaDiagnostics (line 12).
  3. Remove the unused import of sys (line 17).
  4. Remove the unused import of matplotlib.pyplot as plt (line 19).

Additionally, consider grouping related imports and using a consistent import style throughout the file.

Apply this diff to clean up the imports:

-from pyphare.pharein import (
-    ElectromagDiagnostics,
-    FluidDiagnostics,
-    ParticleDiagnostics,
-    InfoDiagnostics,
-)
-from pyphare.pharein import MetaDiagnostics
+from pyphare.pharein import (
+    ElectromagDiagnostics,
+    FluidDiagnostics,
+    InfoDiagnostics,
+)
 from pyphare.pharein import ElectronModel
 from pyphare.simulator.simulator import Simulator, startMPI
 from pyphare.pharein import global_vars as gv
 from pyphare.pharein import LoadBalancer
-import sys
 import numpy as np
-import matplotlib.pyplot as plt
 import matplotlib as mpl
📝 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.

Suggested change
#!/usr/bin/env python3
import pyphare.pharein as ph # lgtm [py/import-and-import-from]
from pyphare.pharein import Simulation
from pyphare.pharein import MaxwellianFluidModel
from pyphare.pharein import (
ElectromagDiagnostics,
FluidDiagnostics,
ParticleDiagnostics,
InfoDiagnostics,
)
from pyphare.pharein import MetaDiagnostics
from pyphare.pharein import ElectronModel
from pyphare.simulator.simulator import Simulator, startMPI
from pyphare.pharein import global_vars as gv
from pyphare.pharein import LoadBalancer
import sys
import numpy as np
import matplotlib.pyplot as plt
import matplotlib as mpl
from tests.simulator.test_advance import AdvanceTestBase
mpl.use("Agg")
from pyphare.cpp import cpp_lib
cpp = cpp_lib()
startMPI()
#!/usr/bin/env python3
import pyphare.pharein as ph # lgtm [py/import-and-import-from]
from pyphare.pharein import Simulation
from pyphare.pharein import MaxwellianFluidModel
from pyphare.pharein import (
ElectromagDiagnostics,
FluidDiagnostics,
InfoDiagnostics,
)
from pyphare.pharein import ElectronModel
from pyphare.simulator.simulator import Simulator, startMPI
from pyphare.pharein import global_vars as gv
from pyphare.pharein import LoadBalancer
import numpy as np
import matplotlib as mpl
from tests.simulator.test_advance import AdvanceTestBase
mpl.use("Agg")
from pyphare.cpp import cpp_lib
cpp = cpp_lib()
startMPI()
🧰 Tools
🪛 Ruff

9-9: pyphare.pharein.ParticleDiagnostics imported but unused

Remove unused import: pyphare.pharein.ParticleDiagnostics

(F401)


12-12: pyphare.pharein.MetaDiagnostics imported but unused

Remove unused import: pyphare.pharein.MetaDiagnostics

(F401)


17-17: sys imported but unused

Remove unused import: sys

(F401)


19-19: matplotlib.pyplot imported but unused

Remove unused import: matplotlib.pyplot

(F401)


test = AdvanceTestBase()


def config():

start_time = 0.0
L = 0.5
Simulation(
time_step=0.010,
final_time=0.15,
# boundary_types="periodic",
cells=(80, 40),
dl=(0.40, 0.40),
refinement="tagging",
max_nbr_levels=1,
nesting_buffer=1,
clustering="tile",
tag_buffer="10",
tagging_threshold=0.4,
hyper_resistivity=0.000,
# hyper_mode="constant",
resistivity=0.001,
diag_options={
"format": "phareh5",
"options": {"dir": "diags_master", "mode": "overwrite"},
},
restart_options={
"dir": "checkpoints",
"mode": "overwrite",
"elapsed_timestamps": [36000, 79000],
},
) # ,"restart_time":start_time }

def density(x, y):
from pyphare.pharein.global_vars import sim

Ly = sim.simulation_domain()[1]
return (
0.4
+ 1.0 / np.cosh((y - Ly * 0.3) / L) ** 2
+ 1.0 / np.cosh((y - Ly * 0.7) / L) ** 2
)

def S(y, y0, l):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Rename ambiguous variable l to improve clarity.

Using l (lowercase 'L') as a variable name can be confused with the digit 1. It's recommended to use a more descriptive name.

Apply this diff to rename the variable:

-def S(y, y0, l):
+def S(y, y0, length):

Also, update all occurrences of l within the function to length:

-return 0.5 * (1.0 + np.tanh((y - y0) / l))
+return 0.5 * (1.0 + np.tanh((y - y0) / length))

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Ruff

72-72: Ambiguous variable name: l

(E741)

return 0.5 * (1.0 + np.tanh((y - y0) / l))
Comment on lines +72 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Rename ambiguous variable for better readability

The variable name l in the S() function is ambiguous and could be confused with the number 1. Consider renaming it to a more descriptive name.

Apply this diff to improve readability:

-def S(y, y0, l):
-    return 0.5 * (1.0 + np.tanh((y - y0) / l))
+def S(y, y0, scale_length):
+    return 0.5 * (1.0 + np.tanh((y - y0) / scale_length))
📝 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.

Suggested change
def S(y, y0, l):
return 0.5 * (1.0 + np.tanh((y - y0) / l))
def S(y, y0, scale_length):
return 0.5 * (1.0 + np.tanh((y - y0) / scale_length))
🧰 Tools
🪛 Ruff

72-72: Ambiguous variable name: l

(E741)


def by(x, y):
from pyphare.pharein.global_vars import sim

Lx = sim.simulation_domain()[0]
Ly = sim.simulation_domain()[1]
sigma = 1.0
dB = 0.1

x0 = x - 0.5 * Lx
y1 = y - 0.3 * Ly
y2 = y - 0.7 * Ly

dBy1 = 2 * dB * x0 * np.exp(-(x0**2 + y1**2) / (sigma) ** 2)
dBy2 = -2 * dB * x0 * np.exp(-(x0**2 + y2**2) / (sigma) ** 2)

return dBy1 + dBy2

def bx(x, y):
from pyphare.pharein.global_vars import sim

Lx = sim.simulation_domain()[0]
Ly = sim.simulation_domain()[1]
sigma = 1.0
dB = 0.1

x0 = x - 0.5 * Lx
y1 = y - 0.3 * Ly
y2 = y - 0.7 * Ly

dBx1 = -2 * dB * y1 * np.exp(-(x0**2 + y1**2) / (sigma) ** 2)
dBx2 = 2 * dB * y2 * np.exp(-(x0**2 + y2**2) / (sigma) ** 2)

v1 = -1
v2 = 1.0
return v1 + (v2 - v1) * (S(y, Ly * 0.3, L) - S(y, Ly * 0.7, L)) + dBx1 + dBx2

def bz(x, y):
return 0.0

def b2(x, y):
return bx(x, y) ** 2 + by(x, y) ** 2 + bz(x, y) ** 2

def T(x, y):
K = 0.7
temp = 1.0 / density(x, y) * (K - b2(x, y) * 0.5)
assert np.all(temp > 0)
return temp

def vx(x, y):
return 0.0

def vy(x, y):
return 0.0

def vz(x, y):
return 0.0

def vthx(x, y):
return np.sqrt(T(x, y))

def vthy(x, y):
return np.sqrt(T(x, y))

def vthz(x, y):
return np.sqrt(T(x, y))

vvv = {
"vbulkx": vx,
"vbulky": vy,
"vbulkz": vz,
"vthx": vthx,
"vthy": vthy,
"vthz": vthz,
"nbr_part_per_cell": 100,
}

MaxwellianFluidModel(
bx=bx,
by=by,
bz=bz,
protons={
"charge": 1,
"density": density,
**vvv,
"init": {"seed": cpp.mpi_rank() + 11},
},
)

ElectronModel(closure="isothermal", Te=0.0)

LoadBalancer(active=True, mode="nppc", tol=0.05, every=1000)

sim = ph.global_vars.sim
dt = 1.0 * sim.time_step
nt = (sim.final_time - start_time) / dt
timestamps = start_time + dt * np.arange(nt)
print(timestamps)

for quantity in ["E", "B"]:
ElectromagDiagnostics(
quantity=quantity,
write_timestamps=timestamps,
)

for quantity in ["density", "bulkVelocity"]:
FluidDiagnostics(
quantity=quantity,
write_timestamps=timestamps,
)

InfoDiagnostics(quantity="particle_count", write_timestamps=timestamps)


def check_hier(hier):
for ilvl, lvl in hier.levels().items():
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Rename unused loop variable ilvl to _ to indicate it's unused.

The loop variable ilvl is not used within the loop body. Renaming it to _ improves readability by indicating it's intentionally unused.

Apply this diff:

-for ilvl, lvl in hier.levels().items():
+for _, lvl in hier.levels().items():
📝 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.

Suggested change
for ilvl, lvl in hier.levels().items():
for _, lvl in hier.levels().items():
🧰 Tools
🪛 Ruff

189-189: Loop control variable ilvl not used within loop body

Rename unused ilvl to _ilvl

(B007)

for patch in lvl.patches:
assert all(patch.box.shape > 5)
return hier
Comment on lines +188 to +192
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Use underscore for unused loop variable

In the check_hier() function, the loop variable ilvl is not used within the loop body. It's a good practice to use an underscore _ for unused variables to make the intention clear.

Apply this diff to improve code clarity:

-def check_hier(hier):
-    for ilvl, lvl in hier.levels().items():
+def check_hier(hier):
+    for _, lvl in hier.levels().items():
         for patch in lvl.patches:
             assert all(patch.box.shape > 5)
     return hier
📝 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.

Suggested change
def check_hier(hier):
for ilvl, lvl in hier.levels().items():
for patch in lvl.patches:
assert all(patch.box.shape > 5)
return hier
def check_hier(hier):
for _, lvl in hier.levels().items():
for patch in lvl.patches:
assert all(patch.box.shape > 5)
return hier
🧰 Tools
🪛 Ruff

189-189: Loop control variable ilvl not used within loop body

Rename unused ilvl to _ilvl

(B007)



def get_time(path, time=None, datahier=None):
if time is not None:
time = "{:.10f}".format(time)
from pyphare.pharesee.hierarchy import hierarchy_from

datahier = hierarchy_from(h5_filename=path + "/EM_E.h5", times=time, hier=datahier)
datahier = hierarchy_from(h5_filename=path + "/EM_B.h5", times=time, hier=datahier)
return datahier


def get_hier(path):
return get_time(path)


def post_advance(new_time):
datahier = check_hier(
get_hier("/home/aunai/Documents/code/phare/phare_jobs/run104/diags_master")
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Avoid hardcoding absolute file paths for better portability.

Using absolute paths like "/home/aunai/Documents/code/phare/phare_jobs/run104/diags_master" makes the code less portable and harder to maintain. Consider using relative paths or configuration variables.

Apply this diff to use a relative path or a variable:

-get_hier("/home/aunai/Documents/code/phare/phare_jobs/run104/diags_master")
+data_directory = "/path/to/diags_master"  # Define a variable or obtain from configuration
+get_hier(data_directory)

This change enhances portability and maintainability of the code.

Committable suggestion was skipped due to low confidence.

)
errors = test.base_test_overlaped_fields_are_equal(datahier, new_time)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Remove assignment to unused variable errors.

The variable errors is assigned but never used, which may lead to confusion.

If the variable is not needed, remove the assignment:

-errors = test.base_test_overlaped_fields_are_equal(datahier, new_time)
+test.base_test_overlaped_fields_are_equal(datahier, new_time)

Alternatively, if errors is significant, consider utilizing it appropriately in the code.

📝 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.

Suggested change
errors = test.base_test_overlaped_fields_are_equal(datahier, new_time)
test.base_test_overlaped_fields_are_equal(datahier, new_time)
🧰 Tools
🪛 Ruff

213-213: Local variable errors is assigned to but never used

Remove assignment to unused variable errors

(F841)



def main():

config()
simulator = Simulator(gv.sim, print_one_line=False, post_advance=post_advance)
simulator.initialize()
simulator.run()


if __name__ == "__main__":
main()
Loading