MinpvProcessor: opt-in thin-cell handling - #1058
Open
hnil wants to merge 2 commits into
Open
Conversation
hnil
added
the
manual:enhancement
An edge-conformal grid has to stay topologically connected, so cells removed by MINPV must be merged geometrically rather than bridged with NNCs, which would create faces with no geometric counterpart. Selects mergeMinPVCells for edge-conformal grids, asserts no MINPV/PINCH/explicit NNC survives, and treats the merged columns as active gaps. No effect unless edge_conformal is set. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
New defaulted parameter thin_cells_as_minpv (default false, behaviour unchanged). When enabled, active cells thinner than z_tolerance are treated like low-pore-volume cells throughout - removal, the column walk, NNC suppression and the gap check - and the merge branch runs for all removed cells, not only low-pv ones. Wired to edge_conformal in processEclipseFormat. Every new condition reduces to the plain pore-volume comparison when the option is off, so the default path is unchanged rather than merely equivalent-looking. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
hnil
force-pushed
the
pr/minpv-thin-cells-optin
branch
from
August 20, 2026 09:50
5078109 to
4e54de6
Compare
hnil
changed the title
Member
|
jenkins build this please |
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in mode to Opm::MinpvProcessor to treat thin active cells (thickness below z_tolerance) as MINPV-like, and wires that behavior into cpgrid::processEclipseFormat via the edge_conformal option to keep edge-conformal grids topologically connected.
Changes:
- Extend
MinpvProcessor::process()with a new defaulted parameterthin_cells_as_minpvto treat thin active cells like low-pore-volume cells. - Adjust MINPV processing logic to apply thin-cell handling across removal/column walk/merge/NNC suppression and gap checks when enabled.
- In
processEclipseFormat, enable MINPV merging and thin-cell-as-MINPV behavior whenedge_conformalis set, and add invariants to ensure no MINPV NNC bridging is produced.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| opm/grid/MinpvProcessor.hpp | Adds the new thin_cells_as_minpv API parameter and documents its intent. |
| opm/grid/MinpvProcessor.cpp | Implements thin-cell-as-MINPV logic across MINPV processing and NNC/gap handling paths. |
| opm/grid/cpgrid/processEclipseFormat.cpp | Wires the new behavior to edge_conformal and enforces edge-conformal invariants around NNC bridging and pinch behavior. |
Suppressed comments (1)
opm/grid/MinpvProcessor.cpp:346
- This NNC “gap check” branch now uses
! (pv < minpvv || ...), which changes semantics forpv == minpvv(it becomes eligible) even whenthin_cells_as_minpvis false. Also, whenthin_cells_as_minpvis enabled (edge-conformal path), this block can still add NNCs, contradicting the later invariant checks that edge-conformal grids must not produce MINPV NNCs.
if (kk < dims_[2] - 1 && (actnum.empty() || actnum[c]) && !((pv[c] < minpvv[c]) || (thin_cells_as_minpv && thickness[c] < z_tolerance)) &&
multz(c) != 0.0)
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+428
to
+436
| auto pinchActive_copy = pinchActive; | ||
| if (edge_conformal) { | ||
| // Edge-conformal grids merged all removed cells geometrically; | ||
| // there must be no NNC bridging, and pinch handling must treat | ||
| // the merged columns as active gaps. | ||
| pinchActive_copy = true; | ||
| assert(nnc_cells[PinchNNC].empty()); | ||
| assert(nnc_cells[ExplicitNNC].empty()); | ||
| } |
| above_active = actnum.empty() || actnum[c_above]; | ||
| above_inactive = !actnum.empty() && !actnum[c_above]; | ||
| auto above_significant_pv = pv[c_above] > minpvv[c_above]; | ||
| auto above_significant_pv = !((pv[c_above] < minpvv[c_above]) || (thin_cells_as_minpv && thickness[c_above] < z_tolerance)); |
Comment on lines
+231
to
+235
| if (edge_conformal) { | ||
| // Merged (not bridged) cells: an edge-conformal grid must not | ||
| // produce artificial faces via MINPV NNCs. | ||
| assert(minpv_result.nnc.empty()); | ||
| } |
| bool c_thin = (thickness[c] <= z_tolerance); | ||
| bool c_thin_inactive = !c_active && c_thin; | ||
| bool c_low_pv_active = pv[c] < minpvv[c] && c_active; | ||
| bool c_low_pv_active = (pv[c] < minpvv[c] && c_active) || (thin_cells_as_minpv && c_thin && c_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.
New defaulted parameter
thin_cells_as_minpv(defaultfalse, behaviour unchanged). When enabled, active cells thinner thanz_toleranceare treated like low-pore-volume cells throughout — removal, the column walk, NNC suppression and the gap check — and the merge branch runs for all removed cells, not only low-pv ones.Every new condition reduces to the plain pore-volume comparison when the option is off, so the default path is unchanged rather than merely equivalent-looking.
processEclipseFormatwires it toedge_conformal. The requirement comes from the geomech/thermal stack: vertex-based discretizations need a conforming, connected grid.Stacked on #1064, which carries the
processEclipseFormathalf. That part is independent, needed for ordinary edge-conformal grids, and can be reviewed without this one. Until it merges the diff here includes it.