Skip to content

Commit 41263ba

Browse files
Issue #1696 Optimize modelsplitter (#1693)
Fixes #1696 # Description This PR implements some of the key insights @Huite found while optimzing the splitting of Models. Although there are some differences in his implementation and this PR. The optimizatiob exists of 2 parts The First part has to do with the way models and packages are split. As @Huite pointed out the current implementation leads to a package being loaded and unloaded several times, which leads to a huge performance penalty because the package needs to be loaded from disk each time. This has been solved by reverting the order of looping. In the original implementation there was a loop over the models, foreach partition a deepcopy was made of the original model. Then afterwards we loop over each model and access each package in the model to determine if it belongs to the same domain as the model. This is expensive. This PR reverts the loop. Frist a shallow copy is made of all the models. Then we loop over each package of the original package and we determine if belongs to once of the submodels. This way we only need the open a package once. The second part is the way the SSM and BUY packages are updated after the splitting has happend. There was a pience of code that looped over all the flow and transport models to determine which models share the same domain (are in the same partition). This is an expensive operation because the grid data has to be loaded into memory over and over. In this PR this has been solved by bringing the update of the SSM and BUY package to the place where that information is already know i.e. the model splitter. This way we can avoid the expensive loading of the grid data. Also by moving it to the modelsplitter we can avoid forcing the load of the grid data manually With these changes the splitting of the mode @Huite provided me takes now around 5m:30s # Checklist <!--- Before requesting review, please go through this checklist: --> - [x] Links to correct issue - [ ] Update changelog, if changes affect users - [x] PR title starts with ``Issue #nr``, e.g. ``Issue #737`` - [ ] Unit tests were added - [ ] **If feature added**: Added/extended example - [ ] **If feature added**: Added feature to API documentation - [ ] **If pixi.lock was changed**: Ran `pixi run generate-sbom` and committed changes --------- Co-authored-by: JoerivanEngelen <joerivanengelen@hotmail.com>
1 parent b0dc733 commit 41263ba

18 files changed

+452
-141
lines changed

docs/api/changelog.rst

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,13 @@ The format is based on `Keep a Changelog`_, and this project adheres to
99
[Unreleased]
1010
------------
1111

12+
Fixed
13+
~~~~~
14+
15+
- Improved performance of :meth:`imod.mf6.Modflow6Simulation.split` for large
16+
models loaded lazily into memory. Reduced a splitting operation of 2 hours to
17+
a few minutes for a test case.
18+
1219
Added
1320
~~~~~
1421

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
from abc import abstractmethod
2+
3+
from imod.common.interfaces.ipackagebase import IPackageBase
4+
from imod.typing import GridDataArray
5+
6+
7+
class IAgnosticPackage(IPackageBase):
8+
"""
9+
Interface for packages for which the data is defined independent of the domain definition.
10+
"""
11+
12+
@abstractmethod
13+
def to_mf6_pkg(
14+
self,
15+
idomain: GridDataArray,
16+
top: GridDataArray,
17+
bottom: GridDataArray,
18+
k: GridDataArray,
19+
validate: bool = False,
20+
strict_validation: bool = True,
21+
) -> IPackageBase:
22+
raise NotImplementedError

imod/common/interfaces/ilinedatapackage.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
from abc import abstractmethod
22
from typing import TYPE_CHECKING
33

4-
from imod.common.interfaces.ipackagebase import IPackageBase
4+
from imod.common.interfaces.iagnosticpackage import IAgnosticPackage
55

66
if TYPE_CHECKING:
77
import geopandas as gpd
88

99

10-
class ILineDataPackage(IPackageBase):
10+
class ILineDataPackage(IAgnosticPackage):
1111
"""
1212
Interface for packages for which the data is defined by lines independent of the domain definition.
1313
"""

imod/common/interfaces/ipackage.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,3 +34,8 @@ def _is_regridding_supported(self) -> bool:
3434
@abstractmethod
3535
def _is_grid_agnostic_package(self) -> bool:
3636
raise NotImplementedError
37+
38+
@property
39+
@abc.abstractmethod
40+
def pkg_id(self) -> str:
41+
raise NotImplementedError

imod/common/interfaces/ipointdatapackage.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
import numpy as np
44
from numpy.typing import NDArray
55

6-
from imod.common.interfaces.ipackagebase import IPackageBase
6+
from imod.common.interfaces.iagnosticpackage import IAgnosticPackage
77

88

9-
class IPointDataPackage(IPackageBase):
9+
class IPointDataPackage(IAgnosticPackage):
1010
"""
1111
Interface for packages for which the data is defined by x and y coordinates independent of the domain definition.
1212
"""

imod/mf6/hfb.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -713,7 +713,7 @@ def to_mf6_pkg(
713713
bottom: GridDataArray,
714714
k: GridDataArray,
715715
validate=True,
716-
strict_hfb_validation=True,
716+
strict_validation=True,
717717
) -> Mf6HorizontalFlowBarrier:
718718
"""
719719
Write package to Modflow 6 package.
@@ -734,7 +734,7 @@ def to_mf6_pkg(
734734
Grid with hydraulic conductivities.
735735
validate: bool
736736
Validate HorizontalFlowBarrier upon calling this method.
737-
strict_hfb_validation: bool
737+
strict_validation: bool
738738
Turn on strict horizontal flow barrier validation.
739739
740740
Returns
@@ -743,7 +743,7 @@ def to_mf6_pkg(
743743
Low level representation of the HFB package as MODFLOW 6 expects it.
744744
"""
745745
validation_context = ValidationSettings(
746-
validate=validate, strict_hfb_validation=strict_hfb_validation
746+
validate=validate, strict_hfb_validation=strict_validation
747747
)
748748

749749
return self._to_mf6_pkg(idomain, top, bottom, k, validation_context)

0 commit comments

Comments
 (0)