Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disentangle Modflow objects as much from the MetaSWAP objects as possible #1255

Open
JoerivanEngelen opened this issue Oct 24, 2024 · 1 comment · May be fixed by #1259
Open

Disentangle Modflow objects as much from the MetaSWAP objects as possible #1255

JoerivanEngelen opened this issue Oct 24, 2024 · 1 comment · May be fixed by #1259
Assignees

Comments

@JoerivanEngelen
Copy link
Contributor

JoerivanEngelen commented Oct 24, 2024

At present we are in a state where the MetaSWAP objects contain Modflow objects. I decided to do this, just for conceptual reasons: The relevant modflow well data is contained in the metaswap sprinkling object. I didn't think this trhough enough, as this creates all kinds of headaches: it means these packages also have to be regridded, dumped, clipped in some way. Also if we want to regrid modflow packages from MetaSWAP objects, this means grid agnostic wells have to be assigned, meaning wells have to be assigned to layer, row, column twice: Once for Modflow, once for MetaSWAP.

It is a lot easier to only ask for Modflow data when it's really needed: Upon calling .write.

See my earlier comment:

Doing some more thinking on this: It is probably better to refactor the MetaSWAP code base in such a way, that the MODFLOW objects are only necessary upon writing, instead of already upon initialization. For example for the Sprinkling package:

Current state:

class Sprinkling:
    def __init__(
        self,
        max_abstraction_groundwater: xr.DataArray,
        max_abstraction_surfacewater: xr.DataArray,
        modflow_wel: WellDisStructured
     ):
        self.well = modflow_wel
...

    def write(
        self,
        directory: Union[str, Path],
        index: np.ndarray,
        svat: xr.DataArray,
    ):
...

Initial idea:

class Sprinkling:
    def __init__(
        self,
        max_abstraction_groundwater: xr.DataArray,
        max_abstraction_surfacewater: xr.DataArray,
        modflow_wel: Well | LayeredWell
     ):
        self.well = modflow_wel
...

    def write(
        self,
        directory: Union[str, Path],
        index: np.ndarray,
        svat: xr.DataArray,
        modflow_dis: mf6.StructuredDiscretication
        modflow_npf: mf6.NodePropertyFlow
    ):
...

Proposed:

class Sprinkling:
    def __init__(
        self,
        max_abstraction_groundwater: xr.DataArray,
        max_abstraction_surfacewater: xr.DataArray,
        mf6_wellname: str,
     ):
        self.mf6_wellname = mf6_wellname
...

    def write(
        self,
        directory: Union[str, Path],
        index: np.ndarray,
        svat: xr.DataArray,
        modflow_wel: Mf6Wel
    ):
...

This propsed approach has the following advantages over the initial idea:

  1. Modflow objects are only used when really necessary. This has the advantage that we can directly use the more low-level Mf6Wel object instead of the grid agnostic Well and LayeredWell packages, as the latter still have to be assigned to cells. We avoid having to conduct this twice.
  2. Mutations of Modflow data, for example regrid_like, do not have to be done twice. In the initial idea, regridding would have to be called once for the MetaSWAP model and once for the Modflow6Simulation, as we do not check wether the Modflow package assigned to the two different models is a copy or the same package for each model. The proposed approach also means no calls to Modflow6Package.regrid_like are done outside the mf6 module, reducing clutter a bit.

The same approach can be taken for the CouplerMapping object in iMOD Python, and the NodeSvatMapping, RechargeSvatMapping, WellSvatMapping objects in primod.

The MetaMod.write method would be the place fetch the Modflow packages and pass them on through to MetaSwapModel.write

Originally posted by @JoerivanEngelen in #728

@JoerivanEngelen
Copy link
Contributor Author

JoerivanEngelen commented Oct 24, 2024

Just a somewhat related frustration: Right now 5 files are required for coupling Modflow models to MetaSWAP: 2 for MetaSWAP, 3 for iMOD Coupler. In an ideal world, the 2 files for MetaSWAP wouldn't be necessary, and the software would receive its cellids to couple to directly from iMOD Coupler.

If this would be the case, the refactor would only affect primod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: 🧐 In Review
Development

Successfully merging a pull request may close this issue.

1 participant