-
Notifications
You must be signed in to change notification settings - Fork 3
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
Comments
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 |
This was referenced Oct 25, 2024
Closed
JoerivanEngelen
added a commit
that referenced
this issue
Nov 1, 2024
Fixes #1255 and #1056 # Description This is part 2 of the metaswap refactoring on the iMOD Python side. The goal of this step was to require a Mf6Wel object at ``write`` instead of at init. For this I was struggling quite a lot with Linkov's principle and the resulting mypy errors. In the end, I resorted to requiring all arguments for the write method for each package. This results in a bit of a weird situation, where a ``None`` has to be provided for the ``mf6_dis`` and ``mf6_wel`` arguments to the ``write`` of an individual no even using these args. I don't think this is a major hickup for users, as they are nearly always writing with ``model.write`` directly, and not writing individual packages manually. Though not perfect, I think this is certainly a big improvement to the current situation. In total, this PR: - Removes the mf6 packages as attributes from ``Sprinkling`` and ``CouplerMapping``. This makes dumping the metaswap packages easier. This affects the files ``imod.msw.sprinkling.py`` and ``imod.msw.coupler_mapping.py``. - Updates the ``Sprinkling`` and ``CouplerMapping`` class to ask MODFLOW6 packages at ``write`` instead of at ``__init__``. This affects ``imod.msw.sprinkling.py``, ``imod.msw.coupler_mapping.py``, and ``imod.msw.model.py`` - Updates the signatures - Implements a ``regrid_like`` for the Sprinkling package. Affects ``imod.msw.sprinkling.py``. - Adds a test to regrid a MetaSWAP model and its coupled MODFLOW6 model. The ``Mf6Wel`` package has to be created manually now after regridding. # Checklist - [x] Links to correct issue - [x] Update changelog, if changes affect users - [x] PR title starts with ``Issue #nr``, e.g. ``Issue #737`` - [x] Unit tests were added - [ ] **If feature added**: Added/extended example
This was referenced Nov 1, 2024
JoerivanEngelen
added a commit
that referenced
this issue
Nov 8, 2024
# Description This merges MetaSWAP refactoring feature branch into master. Fixes merge conflicts. This consolidates the work in #1255 # Checklist <!--- Before requesting review, please go through this checklist: --> - [ ] Links to correct issue - [x] Update changelog, if changes affect users - [ ] PR title starts with ``Issue #nr``, e.g. ``Issue #737`` - [ ] Unit tests were added - [ ] **If feature added**: Added/extended example
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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:
Originally posted by @JoerivanEngelen in #728
The text was updated successfully, but these errors were encountered: