#13863 Roff: Load grid zonation (subgrids) as SUBGRIDS input property#914
Open
#13863 Roff: Load grid zonation (subgrids) as SUBGRIDS input property#914
Conversation
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoLoad ROFF grid zonation as SUBGRIDS input property
WalkthroughsDescription• Load ROFF grid zonation (subgrids) as SUBGRIDS input property • Expand subgrids.nLayers array into per-cell integer property • Surface zone names as category code-names via color-legend • Add comprehensive unit tests for zone value computation Diagramflowchart LR
A["ROFF File<br/>subgrids.nLayers"] -->|"Read array"| B["computeZoneValuesFromSubgrids"]
B -->|"Compute zone values<br/>with K-flip"| C["Per-cell values<br/>in reservoir order"]
C -->|"Create property"| D["SUBGRIDS<br/>INPUT_PROPERTY"]
E["subgrids.names<br/>optional"] -->|"Read zone names"| F["Create color legend"]
F -->|"Attach to result"| D
File Changes1. ApplicationLibCode/FileInterface/RifRoffFileTools.cpp
|
Code Review by Qodo
1. Subgrids handler unreachable
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
604
to
+613
|
|
||
| keywordMapping[QString::fromStdString( keyword )] = newResultName; | ||
| } | ||
| else if ( keyword == "subgrids.nLayers" ) | ||
| { | ||
| if ( !appendZoneIndexPropertyFromSubgrids( eclipseCaseData, reader, keywordMapping ) ) | ||
| { | ||
| RiaLogging::warning( QString( "Unable to import ROFF subgrids zonation from %1" ).arg( fileName ) ); | ||
| } | ||
| } |
There was a problem hiding this comment.
1. Subgrids handler unreachable 🐞 Bug ≡ Correctness
In createInputProperties, the generic "array length == cellCount" import runs before the special-case "subgrids.nLayers" handler, so when nLayers length happens to equal the grid cell count, subgrids.nLayers will be imported as a per-cell property and SUBGRIDS will not be created. This silently misimports zonation for grids where cellCount == numberOfZones (e.g., 1×1×NZ with one layer per zone).
Agent Prompt
### Issue description
`subgrids.nLayers` is handled after the generic `keywordLength == cellCount` check. If `subgrids.nLayers` length equals the grid cell count (possible for small grids / many zones), the code imports it as a per-cell property and never creates the intended `SUBGRIDS` zonation property.
### Issue Context
`subgrids.nLayers` is *not* a per-cell array; it is per-zone (number of layers per zone). It must be excluded from the generic per-cell import path.
### Fix Focus Areas
- ApplicationLibCode/FileInterface/RifRoffFileTools.cpp[553-613]
### Suggested fix
Move the `keyword == "subgrids.nLayers"` branch **above** the `cellCount == keywordLength` branch (or explicitly exclude `subgrids.nLayers` from the generic branch), ensuring `SUBGRIDS` is created regardless of array length coincidences.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+843
to
+852
| RimColorLegendCollection* colorLegendCollection = RimProject::current()->colorLegendCollection; | ||
|
|
||
| int caseId = 0; | ||
| auto* rimCase = caseData->ownerCase(); | ||
| if ( rimCase ) caseId = rimCase->caseId(); | ||
|
|
||
| colorLegendCollection->deleteColorLegend( caseId, resultName ); | ||
| RimColorLegend* colorLegend = colorLegendCollection->createColorLegend( resultName, codeNames ); | ||
| colorLegendCollection->setDefaultColorLegendForResult( caseId, resultName, colorLegend ); | ||
| colorLegendCollection->updateAllRequiredEditors(); |
There was a problem hiding this comment.
2. Null rimproject dereference 🐞 Bug ☼ Reliability
appendZoneIndexPropertyFromSubgrids dereferences RimProject::current()->colorLegendCollection without checking that RiaApplication::instance() (and thus the current project) exists, which can crash during ROFF import in early startup/headless contexts. This new crash path is triggered whenever subgrids.names is present.
Agent Prompt
### Issue description
`appendZoneIndexPropertyFromSubgrids` unconditionally does:
- `RimProject::current()->colorLegendCollection`
If the application singleton or project is not initialized, this dereference can crash.
### Issue Context
`RimProject::current()` is implemented as `RiaApplication::instance()->project()` with no null checks, and `RiaApplication::instance()` is `nullptr` until a `RiaApplication` is constructed.
### Fix Focus Areas
- ApplicationLibCode/FileInterface/RifRoffFileTools.cpp[832-853]
- ApplicationLibCode/ProjectDataModel/RimProject.cpp[234-237]
- ApplicationLibCode/Application/RiaApplication.cpp[152-160]
### Suggested fix
Before accessing `RimProject::current()->colorLegendCollection`, add a defensive guard:
- If `RiaApplication::instance() == nullptr` or `RimProject::current() == nullptr` or `colorLegendCollection == nullptr`, skip legend creation (still import the SUBGRIDS values) and return `true`.
This keeps the data import functional while avoiding a hard crash in non-UI/early-init contexts.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Read the ROFF "subgrids.nLayers" array and expand it into a 1-based per-cell integer property (SUBGRIDS) so ROFF zonation becomes a regular grid property. If a companion "subgrids.names" array is present, zone names are surfaced as category code-names via the existing color-legend flow.
magnesj
force-pushed
the
roff-layer-info
branch
from
0e68c29 to
65cdb0f
Compare
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
1 participant
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.
Read the ROFF "subgrids.nLayers" array and expand it into a 1-based
per-cell integer property (SUBGRIDS) so ROFF zonation becomes a regular
grid property. If a companion "subgrids.names" array is present, zone
names are surfaced as category code-names via the existing color-legend
flow.