Issue #1779 Implementation of MetaSWAP model splitter#1780
Issue #1779 Implementation of MetaSWAP model splitter#1780JoerivanEngelen merged 6 commits intomasterfrom
Conversation
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
Looks good, I've got some comments, of which 2 are reminders to myself to create an issue so you can ignore those.
| from imod.typing import IntArray | ||
|
|
||
|
|
||
| class CouplerMapping(MetaSwapPackage): |
There was a problem hiding this comment.
Note to self: We need to add a ClippablePackage and SplittablePackage interface, just like we have a RegriddingPackage interface. The fact that we need to add IPackageBase here shows how unclear things become without it. I'll open an issue for this.
| f"Missing the following required packages: {missing_packages}" | ||
| ) | ||
|
|
||
| meteo_set = pkg_types_included & set(METEO_PACKAGES) |
| return msw_model | ||
|
|
||
|
|
||
| def msw_add_sprinkling(msw_model): |
imod/msw/model.py
Outdated
| if isinstance(pkg, GridData): | ||
| continue | ||
|
|
||
| if has_meteogrid_copy and isinstance(pkg, MeteoMapping): |
There was a problem hiding this comment.
What case is catched with has_meteogrid_copy?
A model can have a MeteoGrid, a MeteoGridCopy, or none of those.
In case of the latter, an error is thrown with the extra model check you added in this PR upon writing. In case MeteoGrid, an error is thrown as well. So I guess the has_meteogrid_copy doesn't really serve a purpose here?
There was a problem hiding this comment.
Yes, I agree, good one! I removed the has_meteogrid_copy flag.
| sliced_index = sliced_grid_pkg.generate_index_array() | ||
|
|
||
| # Add package to model if it has data in the overlap. | ||
| if bool(sliced_index.any()): |
There was a problem hiding this comment.
Good that you are catching the case where a submodel is entirely outside the active domain. I found the name has_overlap a bit confusing, as I thought at first it meant to overlapping subdomains. Something like is_in_active_domain is clearer here.
There was a problem hiding this comment.
Sure, I changed it to is_in_active_domain!
| ) | ||
| return clipped | ||
|
|
||
| def split( |
There was a problem hiding this comment.
Note to self: Probably good to create a separate place to put splitting logic of this method and the MODFLOW6Simulation method.
|
2 participants
Fixes #1779
Description
Implementation of MetaSWAP model splitter.
Checklist
Issue #nr, e.g.Issue #737pixi run generate-sbomand committed changes