Nested CARFIN: validate and index against the parent LGR - #5256
Open
hnil wants to merge 2 commits into
Open
Conversation
…grid A nested CARFIN (PARENT != GLOBAL) addresses cells in its parent LGR's own refined Cartesian space, so its I/J/K range and the divisibility of NX/NY/NZ must be checked against the parent LGR's dimensions - not the global grid. Previously LgrCollection::addLgr always built the Carfin with the global GridDims, so a nested box whose parent-local extent exceeds the global grid in some direction (e.g. a child reaching deep into a parent refined NZ=9 over a global NZ=3) was wrongly rejected with 'Index values for lgr greater than global grid size'. addLgr now reads the CARFIN PARENT item; for a nested box it looks up the already-defined parent LGR (parent-before-child deck order) and constructs the Carfin with the parent LGR's dimensions and a trivial all-active/identity index map (a refined Cartesian block is fully active). GLOBAL-parent CARFINs are unchanged. A clear error is thrown if the parent LGR is not yet defined. Verified: a nested deck with NEST1 addressing LGR1-local k 2-8 (LGR1 is NZ=9 over the global NZ=3) now parses, builds the grid, and runs the solve to completion (output disabled) on the gridrefined nested-lgr-serial branch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… location
Per review of the bare throw. This is a deck-input error, so it should carry
the keyword location the way the rest of the parsing path does -- the caller
already logs 'CARFIN in {file} line {line}' and this file already includes
OpmInputError.hpp for exactly that.
addLgr now takes the KeywordLocation and throws OpmInputError, so the user
gets the CARFIN record that names the missing parent instead of a bare
std::invalid_argument with no deck context.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Member
|
@hnil I am out of the office for the next two weeks. I will get back to it when I come back. |
hnil
added
the
manual:bugfix
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
2 participants
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 out of #5250 so the serialization fix there can land on its own — akva2 approved that part but was reasonably unsure about the nested semantics, which is all that is here.
LgrCollection::addLgralways built theCarfinagainst the globalGridDims. A nested CARFIN (PARENTother thanGLOBAL) addresses cells in its parent LGR's refined space, so one whose parent-local extent exceeds the global grid — a child in a parent with refined NZ=9 over a global NZ=3 — was rejected with "Index values for lgr greater than global grid size" despite being valid.It now reads
CARFIN::PARENTand, for a non-GLOBALparent, constructs against that parent'sNX/NY/NZ. Requires parent-before-child ordering in the deck; an undefined parent throwsOpmInputErrorcarrying the CARFIN keyword location (the second commit — this was the barethrowakva2 flagged on #5250).No change for
PARENT GLOBAL, which is every existing deck.Depends on nothing; #5250 is independent.