GEOPY-2779: Add Leroi as option for engine inside Plate Simulation application#378
GEOPY-2779: Add Leroi as option for engine inside Plate Simulation application#378
Conversation
…lder with conftest.py added to ensure the PATH variable is accessible in pytest runs).
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
Adds an initial (toggleable) path for running plate simulations via a LeroiAir-based workflow, alongside early interface/options scaffolding and related tests/UI wiring.
Changes:
- Introduces a
use_leroioption (Python + UI JSON) intended to switch plate simulation execution to a LeroiAir driver for TEM forward runs. - Adds initial
leroi_airoptions/interface/driver modules undersimpeg_drivers/plate_simulation/leroi_air/. - Adds new test scaffolding and a pytest fixture to help resolve executables on PATH in IDE-driven runs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/plate_simulation/runtest/leroi_test.py | Adds Leroi-related runtime tests (currently incomplete). |
| tests/plate_simulation/runtest/conftest.py | Adds session fixture to prepend conda Scripts/ to PATH. |
| tests/plate_simulation/leroi_air/options_test.py | Adds assertions around newly introduced LeroiAir options behavior. |
| tests/plate_simulation/leroi_air/interface_test.py | Placeholder for interface tests (header-only). |
| tests/plate_simulation/leroi_air/driver_test.py | Placeholder for driver tests (currently incomplete). |
| tests/plate_simulation/leroi_air/init.py | Provides helper to generate LeroiAirOptions for tests. |
| simpeg_drivers/plate_simulation/options.py | Adds use_leroi flag to plate simulation options. |
| simpeg_drivers/plate_simulation/leroi_air/options.py | Introduces LeroiAirOptions dataclass and derived properties. |
| simpeg_drivers/plate_simulation/leroi_air/interface.py | Adds CFL formatting/writing interface for LeroiAir (scaffolding). |
| simpeg_drivers/plate_simulation/leroi_air/driver.py | Adds a LeroiAir driver stub (currently inconsistent with options/interface). |
| simpeg_drivers/plate_simulation/leroi_air/init.py | Initializes the new leroi_air package (header-only). |
| simpeg_drivers/plate_simulation/driver.py | Wires use_leroi toggle into driver selection logic. |
| simpeg_drivers-assets/uijson/plate_simulation.ui.json | Adds UI control for use_leroi. |
Comments suppressed due to low confidence (1)
tests/plate_simulation/runtest/leroi_test.py:22
test_leroi_runhas no function body, which will raise anIndentationErrorduring test collection. Add a body (e.g., implement assertions or at leastpass/pytest.skip(...)) so the test module imports cleanly.
def test_leroi_run(tmp_path):
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
# Conflicts: # tests/plate_simulation/models/plates_test.py
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def run(self) -> None: | ||
| """Write input, run LeroiAir, and save simulated data to geoh5.""" | ||
| self.interface.write_cfl_file(self.project_path / "LeroiAir.cfl") | ||
| self.run_leroi() | ||
| self.interface.save_to_geoh5( | ||
| outfile=self.project_path / "LeroiAir.out", | ||
| out_group=self.out_group, | ||
| ) |
There was a problem hiding this comment.
LeroiAirDriver.run() passes self.out_group into save_to_geoh5, but out_group defaults to None. If a caller forgets to set it, this will fail with a non-obvious attribute error inside save_to_geoh5. Add an explicit check (and raise a clear exception) when out_group is not set.
There was a problem hiding this comment.
To Copilot's comment, you should validate/set the out_group in the init of the driver. Check the simpeg_drivers.BaseDriver mechanics...
There was a problem hiding this comment.
This is out of date. I'm passing self.options.out_group now which is treated the same as in simpeg_drivers.BaseDriver (ie: it can't be none).
There was a problem hiding this comment.
self.options.out_group can be None, it is typed as such. The base driver uses self.out_group which is validated/created in the init
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…model as required by simpeg runs
Co-authored-by: domfournier <dominiquef@mirageoscience.com>
# Conflicts: # simpeg_drivers/plate_simulation/driver.py # simpeg_drivers/plate_simulation/options.py # simpeg_drivers/utils/utils.py
…aved on plate-simulation mesh and starting_model saved on forward mesh. Fix mesh naming
domfournier
left a comment
There was a problem hiding this comment.
Getting there.
Can you review the comments, but also take a look at the failing test. We should NOT try to run the exe for now, just do it locally.
| """Extract channel data for a single component from a LeroiAir .out file.""" | ||
| lines = Path(outfile).read_text(encoding="utf-8", errors="replace").splitlines() | ||
| data_lines = self._slice_data_lines(lines, self._COMPONENT_ANCHORS[component]) | ||
| return np.array([line.split() for line in data_lines], dtype=float)[:, 4:] |
There was a problem hiding this comment.
This is where the unit conversion should occur if needed.
There was a problem hiding this comment.
Not sure what you intended here. The unit conversion is in time. I handled this by adding back in properties on the LeroiAirOptions for channels, timing_mark, and waveform, and applying the conversion to milliseconds that LeroiAir expects.
There was a problem hiding this comment.
As long as it comes back in SI like SimPEG, then you can ignore.
| layer_thicknesses=[model.overburden_options.thickness, 9999], | ||
| plate_resistivities=[model.plate_options.plate_property], | ||
| plate_geometries=[model.plate_options.geometry], | ||
| magnetic_field="dBdt" if "dBdt" in simulation.data_units else "B", |
There was a problem hiding this comment.
Why not preserving the name and values of data_units?
There was a problem hiding this comment.
it's just a flag to run leroi air in STEP=0 or 1 so I didn't think it was necessary, I simplified to just take a bool.
| def run(self) -> None: | ||
| """Write input, run LeroiAir, and save simulated data to geoh5.""" | ||
| self.interface.write_cfl_file(self.project_path / "LeroiAir.cfl") | ||
| self.run_leroi() | ||
| self.interface.save_to_geoh5( | ||
| outfile=self.project_path / "LeroiAir.out", | ||
| out_group=self.out_group, | ||
| ) |
There was a problem hiding this comment.
To Copilot's comment, you should validate/set the out_group in the init of the driver. Check the simpeg_drivers.BaseDriver mechanics...
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #378 +/- ##
===========================================
+ Coverage 90.26% 90.36% +0.10%
===========================================
Files 132 135 +3
Lines 6469 6765 +296
Branches 814 822 +8
===========================================
+ Hits 5839 6113 +274
- Misses 420 439 +19
- Partials 210 213 +3
🚀 New features to boost your workflow:
|
… to milliseconds if survey.unit is not ms
| def run(self) -> None: | ||
| """Write input, run LeroiAir, and save simulated data to geoh5.""" | ||
| self.interface.write_cfl_file(self.project_path / "LeroiAir.cfl") | ||
| self.run_leroi() | ||
| self.interface.save_to_geoh5( | ||
| outfile=self.project_path / "LeroiAir.out", | ||
| out_group=self.out_group, | ||
| ) |
There was a problem hiding this comment.
self.options.out_group can be None, it is typed as such. The base driver uses self.out_group which is validated/created in the init
| def record_2(self) -> str: | ||
| return self.format_line(["TDFD", "DO3D", "PRFL", "ISTOP"]) |
There was a problem hiding this comment.
Would you mind adding docstrings for all of these. Just a translation of the code from the docs.
| """Extract channel data for a single component from a LeroiAir .out file.""" | ||
| lines = Path(outfile).read_text(encoding="utf-8", errors="replace").splitlines() | ||
| data_lines = self._slice_data_lines(lines, self._COMPONENT_ANCHORS[component]) | ||
| return np.array([line.split() for line in data_lines], dtype=float)[:, 4:] |
There was a problem hiding this comment.
As long as it comes back in SI like SimPEG, then you can ignore.





GEOPY-2779 - Add Leroi as option for engine inside Plate Simulation application