diff --git a/docs/api/changelog.rst b/docs/api/changelog.rst index 82a3f5d96..acadd5334 100644 --- a/docs/api/changelog.rst +++ b/docs/api/changelog.rst @@ -9,6 +9,15 @@ The format is based on `Keep a Changelog`_, and this project adheres to [Unreleased] ------------ +Added +~~~~~ + +- Added ``ignore_time_purge_empty`` argument to + :meth:`imod.mf6.Modflow6Simulation.mask_all_models` and + :meth:`imod.mf6.Modflow6Simulation.clip_box` to consider a package empty if + its first times step is all nodata. This can save a lot of clipping or masking + transient models with many timesteps. + Fixed ~~~~~ diff --git a/imod/common/utilities/mask.py b/imod/common/utilities/mask.py index 7e0809145..b0199cb30 100644 --- a/imod/common/utilities/mask.py +++ b/imod/common/utilities/mask.py @@ -37,9 +37,10 @@ def _validate_coords_mask(mask: GridDataArray) -> None: ) -def _mask_all_models( +def mask_all_models( simulation: ISimulation, mask: GridDataArray, + ignore_time_purge_empty: bool = False, ): _validate_coords_mask(mask) if simulation.is_split(): @@ -54,21 +55,22 @@ def _mask_all_models( for name in modelnames: if is_same_domain(simulation[name].domain, mask): - simulation[name].mask_all_packages(mask) + simulation[name].mask_all_packages(mask, ignore_time_purge_empty) else: raise ValueError( "masking can only be applied to simulations when all the models in the simulation use the same grid." ) -def _mask_all_packages( +def mask_all_packages( model: IModel, mask: GridDataArray, + ignore_time_purge_empty: bool = False, ): _validate_coords_mask(mask) for pkgname, pkg in model.items(): model[pkgname] = pkg.mask(mask) - model.purge_empty_packages() + model.purge_empty_packages(ignore_time=ignore_time_purge_empty) def mask_package(package: IPackage, mask: GridDataArray) -> IPackage: diff --git a/imod/mf6/model.py b/imod/mf6/model.py index ee0de6da6..97c464be5 100644 --- a/imod/mf6/model.py +++ b/imod/mf6/model.py @@ -22,7 +22,7 @@ from imod.common.serializer import EngineType from imod.common.statusinfo import NestedStatusInfo, StatusInfo, StatusInfoBase from imod.common.utilities.clip import clip_box_dataset -from imod.common.utilities.mask import _mask_all_packages +from imod.common.utilities.mask import mask_all_packages from imod.common.utilities.regrid import _regrid_like from imod.common.utilities.schemata import ( concatenate_schemata_dicts, @@ -697,6 +697,7 @@ def clip_box( y_min: Optional[float] = None, y_max: Optional[float] = None, state_for_boundary: Optional[GridDataArray] = None, + ignore_time_purge_empty: bool = False, ): """ Clip a model by a bounding box (time, layer, y, x). @@ -724,6 +725,14 @@ def clip_box( state_for_boundary : optional, Union[xr.DataArray, xu.UgridDataArray] A grids with states that are used to put as boundary values. This model will get a :class:`imod.mf6.ConstantHead`. + ignore_time_purge_empty : bool, default False + Whether to ignore the time dimension when purging empty packages. + Can improve performance when clipping models with many time steps. + Clipping models can cause package data with all nans. These packages + are considered empty and need to be removed. However, checking all + timesteps for each package is a costly operation. Therefore, this + option can be set to True to only check the first timestep. Defaults + to False. Returns ------- @@ -786,7 +795,7 @@ def clip_box( if clipped_boundary_condition is not None: clipped[pkg_name] = clipped_boundary_condition - clipped.purge_empty_packages() + clipped.purge_empty_packages(ignore_time=ignore_time_purge_empty) return clipped @@ -882,6 +891,7 @@ def regrid_like( def mask_all_packages( self, mask: GridDataArray, + ignore_time_purge_empty: bool = False, ): """ This function applies a mask to all packages in a model. The mask must @@ -898,9 +908,12 @@ def mask_all_packages( mask: xr.DataArray, xu.UgridDataArray of ints idomain-like integer array. >0 sets cells to active, 0 sets cells to inactive, <0 sets cells to vertical passthrough + ignore_time_purge_empty: bool, default False + Whether to ignore time dimension when purging empty packages. Can + improve performance when masking models with many time steps. """ - _mask_all_packages(self, mask) + mask_all_packages(self, mask, ignore_time_purge_empty) def purge_empty_packages( self, model_name: Optional[str] = "", ignore_time: bool = False diff --git a/imod/mf6/simulation.py b/imod/mf6/simulation.py index d3c734f5c..976219fbd 100644 --- a/imod/mf6/simulation.py +++ b/imod/mf6/simulation.py @@ -24,7 +24,7 @@ from imod.common.serializer import EngineType from imod.common.statusinfo import NestedStatusInfo from imod.common.utilities.dataclass_type import DataclassType -from imod.common.utilities.mask import _mask_all_models +from imod.common.utilities.mask import mask_all_models from imod.common.utilities.regrid import _regrid_like from imod.common.utilities.version import ( get_version, @@ -1202,6 +1202,7 @@ def clip_box( y_min: Optional[float] = None, y_max: Optional[float] = None, states_for_boundary: Optional[dict[str, GridDataArray]] = None, + ignore_time_purge_empty: Optional[bool] = None, ) -> Modflow6Simulation: """ Clip a simulation by a bounding box (time, layer, y, x). @@ -1233,6 +1234,14 @@ def clip_box( :class:`imod.mf6.ConstantHead`, :class:`imod.mf6.GroundwaterTransportModel` will get a :class:`imod.mf6.ConstantConcentration` package. + ignore_time_purge_empty: optional, bool, default None + Whether to ignore the time dimension when purging empty packages. + Can improve performance when clipping models with many time steps. + Clipping models can cause package data with all nans. These packages + are considered empty and need to be removed. However, checking all + timesteps for each package is a costly operation. Therefore, this + option can be set to True to only check the first timestep. If None, + the value of the validation settings of the simulation are used. Returns ------- @@ -1276,6 +1285,8 @@ def clip_box( raise ValueError( "Unable to clip simulation. Clipping can only be done on simulations that have a single flow model ." ) + if ignore_time_purge_empty is None: + ignore_time_purge_empty = self._validation_context.ignore_time for model_name, model in self.get_models().items(): supported, error_with_object = model._is_clipping_supported() if not supported: @@ -1301,6 +1312,7 @@ def clip_box( y_min=y_min, y_max=y_max, state_for_boundary=state_for_boundary, + ignore_time_purge_empty=ignore_time_purge_empty, ) elif isinstance(value, Package): clipped[key] = value.clip_box( @@ -1398,11 +1410,14 @@ def split( similar shape as a layer in the domain. The values in the array indicate to which partition a cell belongs. The values should be zero or greater. - ignore_time_purge_empty: bool, default None - If True, only the first timestep is validated. This increases - performance for packages with a time dimensions over which changes - of cell activity are not expected. If None, the value of the - validation context is of the simulation is used. + ignore_time_purge_empty: optional, bool, default None + Whether to ignore the time dimension when purging empty packages. + Can improve performance when splitting models with many time steps. + Splitting models can cause package data with all nans. These packages + are considered empty and need to be removed. However, checking all + timesteps for each package is a costly operation. Therefore, this + option can be set to True to only check the first timestep. If None, + the value of the validation settings of the simulation are used. Returns ------- @@ -1688,6 +1703,7 @@ def _has_one_flow_model(self) -> bool: def mask_all_models( self, mask: GridDataArray, + ignore_time_purge_empty: Optional[bool] = None, ) -> None: """ This function applies a mask to all models in a simulation, provided they use @@ -1702,6 +1718,14 @@ def mask_all_models( mask: xr.DataArray, xu.UgridDataArray of ints idomain-like integer array. >0 sets cells to active, 0 sets cells to inactive, <0 sets cells to vertical passthrough + ignore_time_purge_empty : bool, default False + Whether to ignore the time dimension when purging empty packages. + Can improve performance when masking models with many time steps. + Masking models can cause package data with all nans. These packages + are considered empty and need to be removed. However, checking all + timesteps for each package is a costly operation. Therefore, this + option can be set to True to only check the first timestep. Defaults + to False. Examples -------- @@ -1713,7 +1737,9 @@ def mask_all_models( mask should be an idomain-like array, i.e. it should have the same shape as the model and contain integer values. """ - _mask_all_models(self, mask) + if ignore_time_purge_empty is None: + ignore_time_purge_empty = self._validation_context.ignore_time + mask_all_models(self, mask, ignore_time_purge_empty) @classmethod @standard_log_decorator() diff --git a/imod/tests/test_mf6/test_mf6_LHM.py b/imod/tests/test_mf6/test_mf6_LHM.py index 44a8812c9..6fa13a852 100644 --- a/imod/tests/test_mf6/test_mf6_LHM.py +++ b/imod/tests/test_mf6/test_mf6_LHM.py @@ -78,7 +78,7 @@ def cleanup_mf6_sim(simulation: Modflow6Simulation) -> None: pkg.dataset.load() mask = model.domain - simulation.mask_all_models(mask) + simulation.mask_all_models(mask, ignore_time_purge_empty=True) dis = model["dis"] pkgs_to_cleanup = [ diff --git a/imod/tests/test_mf6/test_mf6_mask_simulation.py b/imod/tests/test_mf6/test_mf6_mask_simulation.py index 575693503..6058a49b1 100644 --- a/imod/tests/test_mf6/test_mf6_mask_simulation.py +++ b/imod/tests/test_mf6/test_mf6_mask_simulation.py @@ -9,13 +9,15 @@ from imod.tests.fixtures.mf6_modelrun_fixture import assert_simulation_can_run +@pytest.mark.parametrize("ignore_time", [True, False]) def test_mask_simulation( tmp_path: Path, flow_transport_simulation: imod.mf6.Modflow6Simulation, + ignore_time: bool, ): mask = deepcopy(flow_transport_simulation["flow"].domain) mask.loc[1, 0.5, 15] = 0 - flow_transport_simulation.mask_all_models(mask) + flow_transport_simulation.mask_all_models(mask, ignore_time_purge_empty=ignore_time) assert ( flow_transport_simulation["flow"].domain.sel({"layer": 1, "y": 0.5, "x": 15}) == 0 diff --git a/imod/tests/test_mf6/test_mf6_model_masking.py b/imod/tests/test_mf6/test_mf6_model_masking.py index ed8bbb71a..94355e352 100644 --- a/imod/tests/test_mf6/test_mf6_model_masking.py +++ b/imod/tests/test_mf6/test_mf6_model_masking.py @@ -266,11 +266,13 @@ def test_mask_with_time_coordinate( @pytest.mark.parametrize( "inactivity_marker", [0, -1] ) # 0 = inactive, -1 = vertical passthrough +@pytest.mark.parametrize("ignore_time_purge_empty", [True, False]) def test_mask_structured( tmp_path: Path, structured_flow_model: GroundwaterFlowModel, mask_cells: list[tuple[int, int, int]], inactivity_marker: int, + ignore_time_purge_empty: bool, ): # Arrange # add a well to the model @@ -291,7 +293,9 @@ def test_mask_structured( mask.values[*cell] = inactivity_marker # Act - structured_flow_model.mask_all_packages(mask) + structured_flow_model.mask_all_packages( + mask, ignore_time_purge_empty=ignore_time_purge_empty + ) # Assert unique, counts = np.unique( diff --git a/imod/tests/test_mf6/test_mf6_rch.py b/imod/tests/test_mf6/test_mf6_rch.py index b8b11f64e..ae8c8037f 100644 --- a/imod/tests/test_mf6/test_mf6_rch.py +++ b/imod/tests/test_mf6/test_mf6_rch.py @@ -297,8 +297,16 @@ def test_validate_false(): @pytest.mark.timeout(10, method="thread") def test_ignore_time_validation(): - # Create a large recharge dataset with a time dimension. This is to test the - # performance of the validation when ignore_time_no_data is True. + """ + Create a large recharge dataset with a time dimension. This is to test the + performance of the validation when ignore_time_no_data is True. + NOTE: There currently is no easy way to test the opposite (i.e., + ignore_time_no_data is False, then catch timeout with an pytest xfail + marker), because the timeout will terminate the test run with an error when + using dask instead of a fail. Somewhat relevant issue: + https://github.com/pytest-dev/pytest-timeout/issues/181 + """ + # Arrange rng = dask.array.random.default_rng() layer = [1, 2, 3] template = imod.util.empty_3d(1.0, 0.0, 1000.0, 1.0, 0.0, 1000.0, layer) diff --git a/imod/tests/test_mf6/test_mf6_simulation.py b/imod/tests/test_mf6/test_mf6_simulation.py index 81b9e827c..06a9be30c 100644 --- a/imod/tests/test_mf6/test_mf6_simulation.py +++ b/imod/tests/test_mf6/test_mf6_simulation.py @@ -278,6 +278,14 @@ def test_write_circle_model_twice(circle_model, tmp_path): assert len(diff.right_only) == 0 +@pytest.mark.parametrize("ignore_time_purge_empty", [True, False]) +def test_simulation_clip_box__ignore_time(circle_model, ignore_time_purge_empty): + simulation = circle_model + simulation.clip_box( + y_min=-50, y_max=0, ignore_time_purge_empty=ignore_time_purge_empty + ) + + def test_simulation_clip_box__validation_settings_preserved(circle_model): simulation = circle_model simulation.set_validation_settings(