From 200eeb8e09d0e99b73c0830bc2f51c49d812f4cf Mon Sep 17 00:00:00 2001 From: domfournier Date: Wed, 29 Apr 2026 12:18:55 -0700 Subject: [PATCH 1/4] Accumulate issues with uncertainties --- simpeg_drivers/options.py | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/simpeg_drivers/options.py b/simpeg_drivers/options.py index d1b35a10..2c69c7d6 100644 --- a/simpeg_drivers/options.py +++ b/simpeg_drivers/options.py @@ -17,6 +17,7 @@ from typing import Annotated, Any, ClassVar, Literal import numpy as np +from geoapps_utils import GeoAppsError from geoapps_utils.base import Options from geoh5py.data import ( BooleanData, @@ -458,7 +459,7 @@ def property_group_data(self, property_group: PropertyGroup): k for k in self.data_object.property_groups if k.uid == property_group.uid ) data = { - freq: self.geoh5.get_entity(p)[0].values + freq: self.data_object.get_entity(p)[0].values for freq, p in zip(frequencies, group.properties, strict=False) } return data @@ -615,20 +616,36 @@ def data(self) -> dict[str, dict[float, np.ndarray | None]]: @property def uncertainties(self) -> dict[str, dict[float, np.ndarray | None]]: - """Return dictionary of unceratinty components and associated values.""" + """Return dictionary of uncertainty components and associated values.""" out = {} + flags = [] for k in self.active_components: out[k] = self.component_uncertainty(k) + + for value in out[k].values(): + if value is not None and np.any(np.isnan(value)): + flags.append(f"{k} component has NDV values.") + + if value is not None and np.any(value < 0): + flags.append(f"{k} component has negative values.") + + if flags: + summary = "Issues encountered with uncertainties:\n\n - " + "\n - ".join( + flags + ) + summary += "\n\nPlease review the input values." + raise GeoAppsError(summary) + return out - def component_data(self, component: str) -> np.ndarray | None: + def component_data(self, component: str) -> dict: """Return data values associated with the component.""" data = getattr(self, "_".join([component, "channel"]), None) if isinstance(data, NumericData): data = data.values return {None: data} - def component_uncertainty(self, component: str) -> np.ndarray | None: + def component_uncertainty(self, component: str) -> dict: """ Return uncertainty values associated with the component. From 0500168f21bf9525ad5dda6e63f0c709da9240b8 Mon Sep 17 00:00:00 2001 From: domfournier Date: Wed, 29 Apr 2026 12:19:32 -0700 Subject: [PATCH 2/4] Add unit tests --- tests/run_tests/driver_grav_test.py | 26 ++++++++++++++++++ tests/run_tests/driver_mt_test.py | 42 ++++++++++++++++++++++++++++- 2 files changed, 67 insertions(+), 1 deletion(-) diff --git a/tests/run_tests/driver_grav_test.py b/tests/run_tests/driver_grav_test.py index 41e5400b..985ed06b 100644 --- a/tests/run_tests/driver_grav_test.py +++ b/tests/run_tests/driver_grav_test.py @@ -206,6 +206,32 @@ def test_array_too_large_run( driver.run() +def test_bad_uncertainties( + tmp_path: Path, +): + workpath = tmp_path / f"{__name__}.ui.geoh5" + + with Workspace(workpath) as geoh5: + components = SyntheticsComponents(geoh5) + gz = components.survey.add_data( + {"data": {"values": np.random.randn(components.survey.n_vertices)}} + ) + + # Run the inverse + params = GravityInversionOptions.build( + geoh5=geoh5, + mesh=components.mesh, + topography_object=components.topography, + data_object=gz.parent, + gz_channel=gz, + gz_uncertainty=-1e-4, + starting_model=1e-4, + ) + + with raises(GeoAppsError, match="Issues encountered with uncertainties"): + _ = params.uncertainties + + if __name__ == "__main__": # Full run test_gravity_fwr_run( diff --git a/tests/run_tests/driver_mt_test.py b/tests/run_tests/driver_mt_test.py index e84ea2e5..bebffff3 100644 --- a/tests/run_tests/driver_mt_test.py +++ b/tests/run_tests/driver_mt_test.py @@ -15,7 +15,9 @@ from pathlib import Path import numpy as np +from geoapps_utils.utils.importing import GeoAppsError from geoh5py.workspace import Workspace +from pytest import raises from simpeg_drivers.natural_sources.magnetotellurics import ( MTForwardDriver, @@ -73,7 +75,7 @@ def setup_data(workspace, survey): } } ) - uncertainties[f"{cname} uncertainties"].append(uncert.copy(parent=survey)) + uncertainties[f"{cname} uncertainties"].append(uncert) data_groups = survey.add_components_data(data) uncert_groups = survey.add_components_data(uncertainties) @@ -141,6 +143,44 @@ def test_magnetotellurics_fwr_run( assert geoh5.get_entity("Iteration_0_zyy_real_[0]")[0] is not None +def test_bad_uncertainties( + tmp_path: Path, +): + workpath = ( + tmp_path.parent / "test_magnetotellurics_fwr_run0" / "inversion_test.ui.geoh5" + ) + + with Workspace(workpath) as geoh5: + components = SyntheticsComponents(geoh5) + survey = components.survey.copy(copy_children=False, name="bad_uncertainties") + mesh = components.mesh + topography = components.topography + data_kwargs = setup_data(geoh5, survey) + + for elem in ["uncertainty_zxx_imag_[0]", "uncertainty_zyx_real_[0]"]: + data = survey.get_entity(elem)[0] + vals = data.values + vals[0] = np.nan + data.values = vals + + # Run the inverse + params = MTInversionOptions.build( + geoh5=geoh5, + mesh=mesh, + topography_object=topography, + data_object=survey, + starting_model=100.0, + background_conductivity=100.0, + **data_kwargs, + ) + + with raises(GeoAppsError) as error: + _ = params.uncertainties + + assert "zxx_imag component has NDV values" in str(error.value) + assert "zyx_real component has NDV values" in str(error.value) + + def test_magnetotellurics_run(tmp_path: Path, max_iterations=1, pytest=True): # pass workpath = tmp_path / "inversion_test.ui.geoh5" From 637b8abc0d239b14ed93bc69cb88a71ebfb14c3b Mon Sep 17 00:00:00 2001 From: domfournier Date: Wed, 29 Apr 2026 12:54:39 -0700 Subject: [PATCH 3/4] Move test under context --- tests/run_tests/driver_grav_test.py | 4 +-- tests/run_tests/driver_mt_test.py | 55 +++++++++++++++-------------- 2 files changed, 31 insertions(+), 28 deletions(-) diff --git a/tests/run_tests/driver_grav_test.py b/tests/run_tests/driver_grav_test.py index 985ed06b..4f37bac5 100644 --- a/tests/run_tests/driver_grav_test.py +++ b/tests/run_tests/driver_grav_test.py @@ -228,8 +228,8 @@ def test_bad_uncertainties( starting_model=1e-4, ) - with raises(GeoAppsError, match="Issues encountered with uncertainties"): - _ = params.uncertainties + with raises(GeoAppsError, match="Issues encountered with uncertainties"): + _ = params.uncertainties if __name__ == "__main__": diff --git a/tests/run_tests/driver_mt_test.py b/tests/run_tests/driver_mt_test.py index bebffff3..01414f3e 100644 --- a/tests/run_tests/driver_mt_test.py +++ b/tests/run_tests/driver_mt_test.py @@ -151,34 +151,37 @@ def test_bad_uncertainties( ) with Workspace(workpath) as geoh5: - components = SyntheticsComponents(geoh5) - survey = components.survey.copy(copy_children=False, name="bad_uncertainties") - mesh = components.mesh - topography = components.topography - data_kwargs = setup_data(geoh5, survey) - - for elem in ["uncertainty_zxx_imag_[0]", "uncertainty_zyx_real_[0]"]: - data = survey.get_entity(elem)[0] - vals = data.values - vals[0] = np.nan - data.values = vals - - # Run the inverse - params = MTInversionOptions.build( - geoh5=geoh5, - mesh=mesh, - topography_object=topography, - data_object=survey, - starting_model=100.0, - background_conductivity=100.0, - **data_kwargs, - ) + with Workspace.create(tmp_path / f"{__name__}.geoh5") as ws: + components = SyntheticsComponents(geoh5) + survey = components.survey.copy( + parent=ws, copy_children=False, name="bad_uncertainties" + ) + mesh = components.mesh + topography = components.topography + data_kwargs = setup_data(geoh5, survey) + + for elem in ["uncertainty_zxx_imag_[0]", "uncertainty_zyx_real_[0]"]: + data = survey.get_entity(elem)[0] + vals = data.values + vals[0] = np.nan + data.values = vals + + # Run the inverse + params = MTInversionOptions.build( + geoh5=geoh5, + mesh=mesh, + topography_object=topography, + data_object=survey, + starting_model=100.0, + background_conductivity=100.0, + **data_kwargs, + ) - with raises(GeoAppsError) as error: - _ = params.uncertainties + with raises(GeoAppsError) as error: + _ = params.uncertainties - assert "zxx_imag component has NDV values" in str(error.value) - assert "zyx_real component has NDV values" in str(error.value) + assert "zxx_imag" in str(error.value) + assert "zyx_real" in str(error.value) def test_magnetotellurics_run(tmp_path: Path, max_iterations=1, pytest=True): From d714bdbc3b90e9f9aa4858ec407d03683ee1ae32 Mon Sep 17 00:00:00 2001 From: domfournier Date: Wed, 29 Apr 2026 12:56:29 -0700 Subject: [PATCH 4/4] Copilot suggestions --- simpeg_drivers/options.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/simpeg_drivers/options.py b/simpeg_drivers/options.py index 2c69c7d6..f29f24e5 100644 --- a/simpeg_drivers/options.py +++ b/simpeg_drivers/options.py @@ -17,8 +17,8 @@ from typing import Annotated, Any, ClassVar, Literal import numpy as np -from geoapps_utils import GeoAppsError from geoapps_utils.base import Options +from geoapps_utils.utils.importing import GeoAppsError from geoh5py.data import ( BooleanData, DataAssociationEnum, @@ -622,16 +622,15 @@ def uncertainties(self) -> dict[str, dict[float, np.ndarray | None]]: for k in self.active_components: out[k] = self.component_uncertainty(k) - for value in out[k].values(): - if value is not None and np.any(np.isnan(value)): - flags.append(f"{k} component has NDV values.") - - if value is not None and np.any(value < 0): - flags.append(f"{k} component has negative values.") + for data in out[k].values(): + if np.any(np.isnan(data)) or np.any(data < 0): + flags.append(f"{k} component") + break if flags: - summary = "Issues encountered with uncertainties:\n\n - " + "\n - ".join( - flags + summary = ( + "Issues encountered with uncertainties having NDV or negative values:\n\n - " + + "\n - ".join(flags) ) summary += "\n\nPlease review the input values." raise GeoAppsError(summary)