Skip to content

GEOPY-2490#358

Open
domfournier wants to merge 29 commits intodevelopfrom
GEOPY-2490
Open

GEOPY-2490#358
domfournier wants to merge 29 commits intodevelopfrom
GEOPY-2490

Conversation

@domfournier
Copy link
Collaborator

No description provided.

domfournier and others added 24 commits March 3, 2026 16:03
# Conflicts:
#	simpeg_drivers-assets/uijson/direct_current_2d_forward.ui.json
#	simpeg_drivers-assets/uijson/direct_current_2d_inversion.ui.json
#	simpeg_drivers-assets/uijson/direct_current_batch2d_inversion.ui.json
#	simpeg_drivers-assets/uijson/induced_polarization_2d_forward.ui.json
#	simpeg_drivers-assets/uijson/induced_polarization_2d_inversion.ui.json
#	simpeg_drivers-assets/uijson/induced_polarization_batch2d_inversion.ui.json
@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

❌ Patch coverage is 94.94585% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.84%. Comparing base (b737e1f) to head (71e23c8).

Files with missing lines Patch % Lines
simpeg_drivers/utils/regularization.py 63.63% 1 Missing and 3 partials ⚠️
simpeg_drivers/utils/utils.py 92.15% 3 Missing and 1 partial ⚠️
simpeg_drivers/options.py 84.61% 0 Missing and 2 partials ⚠️
simpeg_drivers/utils/nested.py 94.11% 1 Missing and 1 partial ⚠️
simpeg_drivers/utils/synthetics/driver.py 93.54% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #358      +/-   ##
===========================================
- Coverage    91.26%   90.84%   -0.42%     
===========================================
  Files          120      112       -8     
  Lines         6592     6295     -297     
  Branches       799      775      -24     
===========================================
- Hits          6016     5719     -297     
- Misses         381      396      +15     
+ Partials       195      180      -15     
Files with missing lines Coverage Δ
simpeg_drivers/__init__.py 88.88% <ø> (ø)
simpeg_drivers/components/data.py 92.85% <100.00%> (+0.34%) ⬆️
...drivers/components/factories/directives_factory.py 97.48% <100.00%> (+0.83%) ⬆️
...drivers/components/factories/simulation_factory.py 98.80% <100.00%> (ø)
simpeg_drivers/components/models.py 90.85% <100.00%> (-0.30%) ⬇️
simpeg_drivers/components/topography.py 93.65% <100.00%> (ø)
simpeg_drivers/driver.py 85.00% <100.00%> (ø)
simpeg_drivers/electricals/base_2d.py 100.00% <100.00%> (ø)
...lectricals/direct_current/two_dimensions/driver.py 100.00% <100.00%> (ø)
...ectricals/direct_current/two_dimensions/options.py 100.00% <100.00%> (ø)
... and 12 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@benk-mira benk-mira left a comment

Choose a reason for hiding this comment

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

Looks good, nice use of utilities for separation of concerns and documenting the drape model handling! Couple suggestions

@classmethod
def deprecated_pseudo(cls, data: dict):
if "pseudo 3d" in data.get("inversion_type", ""):
logger.warning(
Copy link
Contributor

Choose a reason for hiding this comment

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

Use DeprecationWarning instead, something like:

Image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not suggesting logger.warn, but warnings.warn so that a proper DecprecationWarning is raised.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything is done through logger at this point. @sebhmg can you weigh in here?

actives = prisms[:, -1] != 1

nu_layers = np.unique(prisms[:, -1])
nu_layers = np.unique(prisms[actives, -1])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I saw the same operation earlier.. pull out as a utility?

Copy link
Collaborator Author

@domfournier domfournier Mar 11, 2026

Choose a reason for hiding this comment

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

Here I need to logic mask, the other the reduced prisms. I really want to avoid maintaining single line functions

Copy link
Contributor

Choose a reason for hiding this comment

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

Fair enough if the operations are different, I don't get your comment about maintaining single line functions. Whats difficult to maintain?

Copy link
Collaborator Author

@domfournier domfournier Mar 13, 2026

Choose a reason for hiding this comment

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

We get lost in a myriad of sub calls, and technically we would have to add unit test for individual ones. If it's just an array operation and truly reusable, we can just rely on numpy to return operator. Flatter repos is always better.

Copilot AI review requested due to automatic review settings March 13, 2026 22:30
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR (GEOPY-2490) refactors the 2D DC/IP inversion workflow by consolidating the "batch 2D" (pseudo-3D) approach into the standard 2D driver. Instead of running each survey line independently via a LineSweepDriver, the 2D drivers now internally handle multi-line surveys by creating a merged DrapeModel mesh and splitting tiles per line.

Changes:

  • Removed the LineSweepDriver, BaseBatch2DDriver, and all pseudo-3D option/driver classes, redirecting pseudo-3D inversion types to the 2D drivers with deprecation warnings.
  • Introduced Base2DDriver and Base2DOptions in a new electricals/base_2d.py that handles DrapeModel mesh creation per line and line-based tiling.
  • Refactored mesh/survey utilities (drape_2_tensor, get_drape_model, compute_alongline_distance, get_parts_from_electrodes, create_mesh_by_line_id) and updated nested mesh creation to support 2D tensor meshes.

Reviewed changes

Copilot reviewed 53 out of 53 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
simpeg_drivers/electricals/base_2d.py New base class for 2D DC/IP drivers and options with pseudo-3D deprecation handling
simpeg_drivers/electricals/driver.py Removed old Base2DDriver and BaseBatch2DDriver
simpeg_drivers/electricals/options.py Removed; IPModelOptions moved to simpeg_drivers/options.py
simpeg_drivers/line_sweep/driver.py Removed LineSweepDriver
simpeg_drivers/options.py Added IPModelOptions, made LineSelectionOptions fields optional, updated gradient orientation handling
simpeg_drivers/utils/utils.py Refactored drape_2_tensor for ghost prism handling, moved compute_alongline_distance, removed truncate_locs_depths
simpeg_drivers/utils/surveys.py Added get_parts_from_electrodes, create_mesh_by_line_id, and related helpers
simpeg_drivers/utils/nested.py Added create_nested_2d_tensor for 2D line-based tiling
simpeg_drivers/components/data.py Refactored parts detection, drape location handling, line ID management
simpeg_drivers/driver.py Updated tiling to split per line for 2D; removed SweepParams references
simpeg_drivers/__init__.py Redirected pseudo-3D driver map entries to 2D drivers
simpeg_drivers/utils/regularization.py direction_and_dip now returns Data objects instead of arrays
simpeg_drivers/components/models.py Apply np.deg2rad to gradient direction/dip
simpeg_drivers/components/factories/* Updated simulation/directive factories for 2D changes
simpeg_drivers/utils/synthetics/* Refactored mesh factory, removed tensor mesh module, updated driver caching
simpeg_drivers-assets/uijson/* Removed batch2D UI JSONs, updated 2D UI JSONs with optional line selection
tests/run_tests/* Updated tests for new 2D workflow, removed batch2D tests

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

"Please use the non-batch classes instead. Results may be affected.",
)
data["inversion_type"] = data["inversion_type"].replace("pseudo 3d", "2d")
data["line_selection"]["line_id"] = None
Comment on lines +205 to +207
np.testing.assert_almost_equal(np.nanmin(model.values[:2369]), 1e-3, decimal=3)
np.testing.assert_almost_equal(np.nanmin(model.values[-2368:]), 1e-3, decimal=3)
assert np.nanmax(model.values[2368:-2368]) > 1e-3

:param centers: n x 3 array of cell center locations

:returns: n-1 array of cell widths
Comment on lines +329 to +330
if "pseudo" in inversion_type:
pass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants