Issue #1835 sprinkling from ipf - #1883
Conversation
…ng, and throw TypeError when format not supported by class
…groundwater = 0, and properly order the columns of the dataframe
…. Not all of them succeed yet.
There was a problem hiding this comment.
Pull request overview
Adds experimental MetaSWAP “sprinkling from IPF” support by introducing a point-based sprinkling package, while keeping the legacy Sprinkling API stable via deprecation and a new SprinklingGrid implementation. This also extends MF6 well conversion from iMOD5 CAP point data and threads stable well IDs through Mf6Wel to support coupling.
Changes:
- Introduce
msw.SprinklingPoints(IPF-based n:1 coupling) andmsw.SprinklingGrid; deprecatemsw.Sprinkling. - Extend iMOD5 CAP → MF6 well conversion to support point (IPF) inputs and propagate well
idintoMf6Wel. - Refactor/expand sprinkling-related tests and modernize some
.valuesusage to.to_numpy()/ scalar enforcement.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| imod/tests/test_msw/test_sprinkling.py | Refactors/expands sprinkling tests to cover grid + new point-based sprinkling and updated API names. |
| imod/tests/test_msw/test_coupler_mapping.py | Updates helper to construct Mf6Wel with the new id argument. |
| imod/tests/test_mf6/test_mf6_wel.py | Updates CAP point-data test to validate LayeredWell.from_imod5_cap_data() now supports IPF. |
| imod/tests/test_mf6/test_mf6_wel_lowlvl.py | Updates low-level Mf6Wel tests to pass id. |
| imod/tests/fixtures/msw_model_fixture.py | Switches fixture sprinkling package to SprinklingGrid and passes well IDs to Mf6Wel. |
| imod/tests/fixtures/imod5_cap_data.py | Adjusts CAP sprinkling point fixture contents/column ordering. |
| imod/msw/utilities/imod5_converter.py | Adds typed helpers and conversion logic for sprinkling-from-points vs sprinkling-from-grids. |
| imod/msw/sprinkling.py | Introduces SprinklingBase, implements SprinklingGrid + new SprinklingPoints, and deprecates Sprinkling. |
| imod/msw/regrid/regrid_schemes.py | Adds SprinklingPointsRegridMethod for regridding the points-based sprinkling mapping grid. |
| imod/msw/model.py | Dispatches sprinkling import based on grid vs points and adjusts package-key resolution to remain compatible with legacy Sprinkling type usage. |
| imod/msw/init.py | Exposes SprinklingGrid/SprinklingPoints and keeps deprecated Sprinkling in the public API. |
| imod/mf6/wel.py | Replaces some .values usages, enforces scalar settings safely, and makes from_imod5_cap_data target discretization optional for point inputs. |
| imod/mf6/utilities/imod5_converter.py | Implements CAP point-data well extraction and makes target_dis optional (required only for grid inputs). |
| imod/mf6/mf6_wel_adapter.py | Adds id as optional/extra dataset data for Mf6Wel to support downstream well-to-sprinkling mapping. |
| docs/api/msw.rst | Updates MSW API docs to list SprinklingGrid and SprinklingPoints. |
| docs/api/changelog.rst | Adds changelog entries for new sprinkling-from-points support and sprinkling deprecation/rename. |
Suppressed comments (1)
imod/msw/sprinkling.py:506
- The TypeError message still directs users to
imod.msw.Sprinkling.from_imod5_data(), but the correct non-point entry point isSprinklingGrid.from_imod5_data()(withSprinklingbeing deprecated).
Unsupported format for artificial_recharge_layer: expected a
DataFrame for point data (IPF), got a grid (IDF). Call
imod.msw.Sprinkling.from_imod5_data() instead.
"""
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ClaireDons
left a comment
There was a problem hiding this comment.
Nice work, that's quite a significant added functionality! Just a few comments for now, mainly for clarity (for me).
| # fmt: on | ||
| case_data.x_p = [2.0, 2.0, 2.0] | ||
| case_data.y_p = [3.0, 2.0, 1.0] | ||
| case_data.layer_p = [1, 2, 3] |
There was a problem hiding this comment.
Does the case were the layer is equal to zero need to be tested? (I think that's a possibility?)
There was a problem hiding this comment.
Very good point! I added some test cases for this, it turned out this required some fixes, and while I was at it, I realized that I could reduce the lengthy _render method a lot, as I was basically conducting the same logic twice in different ways for points outside the svat grid and when layer = 0
There was a problem hiding this comment.
I enjoyed looking at how you set up the tests, some docstrings for the grid cases (like for the point cases) would be nice to make it easier to follow what the cases are.
There was a problem hiding this comment.
Fair point, I've added docstrings to these cases
ClaireDons
left a comment
There was a problem hiding this comment.
LGTM, glad the review was helpful!
…rds to points falling outside the svat grid being checked on a per subunit basis
|



Fixes #1835
Description
Adds experimental support for
SprinklingPointsclassChanges the following things:
SprinklingPointsclass, to specify a n:1 coupling of grid cells to points.Sprinklingclass toSprinklingGridclass, deprecateSprinklingclass. I had to keep the API stable asprimodrelies on the existence of theSprinklingclass.primodrelies on a call toMetaSwapModel.get_pkg_type(Sprinkling), I had to modify the code somewhat, so that it returns the key of the sprinkling package regardless whether it is aSprinklingPointsor aSprinklingGridLayeredWell.from_imod5_cap_datato construct a MODFLOW6 well package from the iMOD5 data under the CAP header.idto theMf6Welclass' dataset. This is used to map MODFLOW6 wells to MetaSWAP wells. It can furthermore be used in the future to add as auxiliary variable in well files in the future to have some metadata in the MODFLOW6 well file.imod/tests/test_msw/test_sprinkling.py: Separate the test cases from the tests..valuestoto_numpy().values[()]to the saferenforce_scalar_dataThis still has to be tested whether it works with the Peelvenen model, I'll do that in follow-up issue #1908 , where I'll also add a larger regression/user_acceptance test for our Weekly tests.
Checklist
Issue #nr, e.g.Issue #737pixi run generate-sbomand committed changes