processEclipseFormat: merge removed cells on edge-conformal grids - #1064
Conversation
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>
|
jenkins build this please |
There was a problem hiding this comment.
Pull request overview
This PR updates CPGrid Eclipse-format processing to support edge-conformal grids by merging MINPV-removed cells geometrically (instead of bridging via NNCs), and by enforcing that edge-conformal grids do not rely on PINCH or explicit NNC connections that would create non-geometric faces.
Changes:
- Wire
edge_conformalto enable geometric merging of MINPV-removed cells viaMinpvProcessor(mergeMinPVCells). - Add edge-conformal invariants intended to ensure no MINPV/PINCH/explicit NNC bridging remains.
- Treat merged columns as active gaps by forcing
pinchActiveduring grid construction whenedge_conformalis enabled.
Suppressed comments (3)
opm/grid/cpgrid/processEclipseFormat.cpp:216
- When
edge_conformalis enabled,mergeMinPVCellsavoids MINPV NNCs, sominpv_result.nnccan be empty even thoughMinpvProcessor::process()still modifiedzcornData(collapsed/merged cells). Guardingthis->zcorn = zcornData;only onminpv_result.nncrisks losing the updated ZCORN needed for correct EclipseGrid output.
This issue also appears in the following locations of the same file:
- line 230
- line 431
poreVolume, ecl_grid.getMinpvVector(), actnumData, mergeMinPVCells,
zcornData.data(), nogap, pinchOptionALL,
permZ, multZ, tolerance_unique_points);
if (!minpv_result.nnc.empty()) {
this->zcorn = zcornData;
opm/grid/cpgrid/processEclipseFormat.cpp:234
- This
assert(minpv_result.nnc.empty())is a release-build no-op, so anedge_conformalgrid could silently proceed while still having MINPV/PINCH NNCs (MinpvProcessor can also generate NNCs for vertical gaps unrelated to MINPV). Ifedge_conformalrequires no NNCs, enforce it with a runtime check and a clear error message.
if (edge_conformal) {
// Merged (not bridged) cells: an edge-conformal grid must not
// produce artificial faces via MINPV NNCs.
assert(minpv_result.nnc.empty());
}
opm/grid/cpgrid/processEclipseFormat.cpp:435
- These
assert()s are compiled out in release builds, soedge_conformalgrids could still be built with PINCH or explicit NNCs even though the comment states they must not. Convert the invariant into a runtime check so invalid decks/configurations fail deterministically with an actionable message.
// the merged columns as active gaps.
pinchActive_copy = true;
assert(nnc_cells[PinchNNC].empty());
assert(nnc_cells[ExplicitNNC].empty());
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
bska
left a comment
There was a problem hiding this comment.
Very good. I'll merge into master.
An edge-conformal grid has to stay topologically connected. Cells removed by MINPV must therefore be merged geometrically rather than bridged with NNCs, which would create faces with no geometric counterpart — vertex-based discretizations cannot use those.
So for
edge_conformalgrids this selectsmergeMinPVCells, asserts that no MINPV, PINCH or explicit NNC survives, and treats the merged columns as active gaps.Nothing changes unless
edge_conformalis set.Split out of #1058 so it can be reviewed on its own: that PR also adds an opt-in thin-cell mode to
MinpvProcessor, which is a separate question and not needed for ordinary grids. #1058 is now stacked on this one.