feat(Mf6Splitter): optionally split a structured model into DISV models - #2820
Open
jdhughes-dev wants to merge 6 commits into
Open
feat(Mf6Splitter): optionally split a structured model into DISV models#2820jdhughes-dev wants to merge 6 commits into
jdhughes-dev wants to merge 6 commits into
Conversation
split_model() and split_multi_model() take to_disv, which writes DISV models from a structured model instead of the structured bounding box of each part. A DISV model carries only the cells assigned to it, and stacks of cells that are inactive in every layer are excluded, so a model no longer pads out to a bounding box full of inactive cells. to_disv is ignored with a warning for a DISV model and is an error for a DISU model. The grid of a model is built one model at a time from the new StructuredGrid.get_cell_iverts(), so peak memory scales with the largest part rather than with the parent grid. save_node_mapping() records the grid type of the split models so a structured to DISV mapping round trips. Closes modflowpy#2816
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2820 +/- ##
===========================================
+ Coverage 55.5% 73.5% +17.9%
===========================================
Files 644 659 +15
Lines 124135 132344 +8209
===========================================
+ Hits 68947 97303 +28356
+ Misses 55188 35041 -20147
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR enhances Mf6Splitter to optionally convert structured (DIS) models into DISV (vertex) models during splitting, so each split model contains only its assigned active cells (excluding cell stacks inactive in every layer) while preserving the ability to reconstruct outputs and round-trip saved node mappings.
Changes:
- Add
to_disvoption tosplit_model()andsplit_multi_model()to write DISV models when splitting a structured model, with validation/warnings for incompatible grid types. - Add
StructuredGrid.get_cell_iverts()and use it to build DISVvertices/cell2dper split model with reduced peak memory usage. - Extend node-mapping save/load to record
new_grid_type, and add new automated tests for structured→DISV splitting, idomain handling, geometry, and reconstruction.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| flopy/mf6/utils/model_splitter.py | Implements to_disv workflow, DIS→DISV remapping, grid-type persistence in node maps, and related reconstruction adjustments. |
| flopy/discretization/structuredgrid.py | Adds StructuredGrid.get_cell_iverts() and reuses it for structured iverts generation. |
| autotest/test_model_splitter.py | Adds coverage for structured→DISV splitting behavior, idomain exclusion rules, reconstruction, geometry, and save/load mapping. |
| autotest/test_grid.py | Adds unit test coverage for StructuredGrid.get_cell_iverts(). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+2021
to
+2025
| if nodes is None: | ||
| nodes = np.arange(self.ncpl, dtype=int) | ||
| else: | ||
| nodes = np.atleast_1d(nodes) | ||
|
|
Contributor
Author
There was a problem hiding this comment.
Fixed in e98301e. nodes is now np.ravel(np.asarray(nodes, dtype=int)), which handles an array of any shape and coerces floats. The ravel returns a view and asarray() is a no-op for an int array, so the node array is not copied on the path that matters, where get_cell_iverts() is called once per split model. The (n, 1) and ndarray cases were added to test_get_cell_iverts.
np.atleast_1d() left a node array with a trailing dimension, for example shape (n, 1), two dimensional, and the vertex arithmetic then failed to broadcast into the one dimensional result rows. Ravel the node numbers and coerce them to int so an array of any shape works as documented. The ravel is a view and asarray() is a no-op for an int array, so the array is not copied.
switch_models() already updates the modelgrid, so caching a second copy of its grid type is redundant and goes stale when the model is switched. Read self._modelgrid.grid_type where the grid type of the parent model is needed, and leave switch_models() alone. Also cover reconstruct_recarray() for a vertex model, which had no test.
jdhughes-dev
changed the title
… to_disv The advanced packages remap cellids through _new_node_to_cellid(), which returns a layer and node pair for a DISV model, and no test exercised that path. Split the lake2tr model, which has LAK, SFR, MVR, observation, and array based recharge and evapotranspiration packages, to DISV models. Run the transient array test for both grid types so the transient arrays and stress period data are covered as well.
Split a GWF-GWT simulation into DISV models and check that both models of each pair are DISV and that the concentrations reconstruct to the original model.
…o_disv A cell with an idomain of -1 passes flow vertically and is not inactive, so its stack is carried into the DISV model and its idomain is kept. Add passthrough cells to the idomain test and compare heads only where cells are active.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
split_model()andsplit_multi_model()taketo_disv, which writes DISV models from astructured model instead of the structured bounding box of each part
every layer are excluded
to_disvis ignored with a warning for a DISV model and is an error for a DISU modelStructuredGrid.get_cell_iverts(); each model grid is built from it one model at a time,so peak memory scales with the largest part rather than with the parent grid
save_node_mapping()records the grid type of the split models so a structured to DISV mappinground trips
Closes #2816