Issue #366: make mask method available to MetaSWAP model class#1871
Issue #366: make mask method available to MetaSWAP model class#1871rleander73 wants to merge 6 commits into
Conversation
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
I have some comments on the public API, after which we have a very useful method!
| # Check that the mask has been applied correctly to each package | ||
| for pkgname, pkg in msw_model.items(): | ||
| if isinstance(pkg, msw.meteo_mapping.PrecipitationMapping): | ||
| continue # Skip PrecipitationMapping package for this test |
There was a problem hiding this comment.
Why are we skipping the PrecipitationMapping? The actual mappings are derived upon writing, if I recall correctly, so I think these objects should be identical? Or am I missing something?
|
|
||
| Parameters | ||
| ---------- | ||
| msw_active: MetaSwapActive, dictionary of xr.DataArray |
There was a problem hiding this comment.
MetaSwapActive currenlty is a developer utility, not public API.
Adding extra dataclasses to the API adds extra entry barriers for users, which I like to avoid: They need to import additional python objects, which they need to look up in the docs.
The "all" mask usually is an .any(dim="subunit"), so I think we can derive that for the user, I would do something like:
def mask_all_packages(self, mask: GridDataArray):
...
mask_all = mask.any(dim="subunit")
msw_active = MetaSwapActive(per_subunit=mask, all=mask_all)
...
|
| nsub = 1 | ||
| for pkg in self.values(): | ||
| if "subunit" in pkg.dataset.dims: | ||
| nsub = pkg.dataset.dims["subunit"] | ||
| break |
There was a problem hiding this comment.
Nice! Please move this to a helper method, it might come of use to call in other methods as well.
JoerivanEngelen
left a comment
JoerivanEngelen
left a comment
There was a problem hiding this comment.
Looks good, I have 2 minor comments, than this is ready to merge!
| Parameters | ||
| ---------- | ||
| mask: GridDataArray | ||
| idomain-like integers. >0 sets cells to active, 0 sets cells to inactive, | ||
| mask is applied on a per-subunit basis if the package has a subunit dimension. | ||
| If the package does not have a subunit dimension, the mask is applied to all subunits of the package. |
There was a problem hiding this comment.
| Parameters | |
| ---------- | |
| mask: GridDataArray | |
| idomain-like integers. >0 sets cells to active, 0 sets cells to inactive, | |
| mask is applied on a per-subunit basis if the package has a subunit dimension. | |
| If the package does not have a subunit dimension, the mask is applied to all subunits of the package. | |
| Parameters | |
| ---------- | |
| mask: GridDataArray | |
| idomain-like integers. >0 sets cells to active, 0 sets cells to inactive, | |
| mask is applied on a per-subunit basis if the mask grid has a subunit dimension. | |
| If the package does not have a subunit dimension, the mask grid is applied to all subunits of the package. |
3 participants
Fixes #366
Description
Checklist
Issue #nr, e.g.Issue #737pixi run generate-sbomand committed changes