Skip to content

Conversation

@magnesj
Copy link
Owner

@magnesj magnesj commented Oct 14, 2025

PR Type

Enhancement, Bug fix, Tests


Description

  • Introduces type-safe cell indexing with ReservoirCellIndex to prevent index confusion

  • Adds BCCON/BCPROP keywords for OPM Flow boundary conditions in sector models

  • Fixes active index usage for radial LGR grids and combo box focus events

  • Refactors statistics tools from RiaStatisticsTools to RigStatisticsTools for better organization


Diagram Walkthrough

flowchart LR
  A["Type-safe indexing"] --> B["ReservoirCellIndex"]
  C["Boundary conditions"] --> D["BCCON keyword"]
  C --> E["BCPROP keyword"]
  F["Statistics refactor"] --> G["RigStatisticsTools"]
  H["Bug fixes"] --> I["LGR indexing"]
  H --> J["ComboBox focus"]
Loading

File Walkthrough

Relevant files
Enhancement
18 files
RigTypeSafeIndex.h
Introduces type-safe index wrapper for reservoir cells     
+76/-0   
RigActiveCellInfo.h
Updates active cell indices to use type-safe index             
+8/-6     
RigActiveCellInfo.cpp
Implements type-safe index in active cell storage               
+2/-2     
RigEclipseResultTools.cpp
Adds BCCON generation and updates border result calculation
+104/-24
RigEclipseResultTools.h
Declares border cell face structure and generation function
+10/-0   
RifOpmFlowDeckFile.cpp
Implements BCCON and BCPROP keyword generation for OPM     
+132/-0 
RifOpmFlowDeckFile.h
Declares methods for boundary condition keyword generation
+12/-0   
RigStatisticsTools.h
Renames and relocates statistics tools class                         
+3/-3     
RigStatisticsTools.cpp
Implements renamed statistics tools                                           
+2/-2     
RigStatisticsMath.h
Updates references to renamed statistics tools                     
+5/-5     
RigStatisticsMath.cpp
Updates statistics calculations with new class name           
+5/-5     
RimFishbones.cpp
Adds unit text for lateral diameter field                               
+4/-0     
RicDuplicateJobFeature.cpp
Implements job duplication feature                                             
+73/-0   
RicDuplicateJobFeature.h
Declares job duplication command feature                                 
+33/-0   
RicRunJobFeature.cpp
Updates run job icon and text                                                       
+2/-2     
RimcGridView.cpp
Adds API for exporting cell visibility                                     
+82/-0   
RimcGridView.h
Declares visible cells internal method                                     
+42/-0   
view.py
Implements Python API for cell visibility export                 
+34/-0   
Tests
3 files
RigEclipseResultTools-Test.cpp
Adds comprehensive tests for border cell and BCCON             
+203/-0 
RifOpmFlowDeckFile-Test.cpp
Adds BCPROP keyword generation test                                           
+118/-0 
test_view.py
Adds Python tests for visible cells functionality               
+64/-0   
Bug fix
7 files
cafPdmUiComboBoxEditor.cpp
Fixes focus out event handling for combo box                         
+59/-51 
cafPdmUiComboBoxEditor.h
Declares CustomQComboBox class and focus out slot               
+24/-0   
RigResultAccessorFactory.cpp
Fixes active index usage for radial LGR grids                       
+20/-3   
RimFishbonesCollection.cpp
Fixes default value overwrite in fishbones creation           
+3/-3     
RimFileSummaryCase.cpp
Skips non-existing files before importing summary data     
+26/-14 
RiuViewer.cpp
Changes Z-scale parameter type to double                                 
+1/-1     
RiuViewer.h
Updates Z-scale method signature                                                 
+1/-1     
Miscellaneous
1 files
ResInsightVersion.cmake
Updates version to 2025.9.3-dev.01                                             
+2/-2     
Additional files
32 files
ResInsight.qrc +3/-1     
CMakeLists_files.cmake +0/-2     
RicMswValveAccumulators.cpp +2/-2     
RicWellPathExportCompletionDataFeatureImpl.cpp +2/-2     
RicExportEclipseSectorModelFeature.cpp +2/-2     
CMakeLists_files.cmake +2/-0     
RicNewOpmFlowJobFeature.cpp +4/-3     
RicNewOpmFlowJobFeature.h +4/-4     
CMakeLists.txt +1/-1     
RimStatisticsContourMap.cpp +9/-9     
RimCorrelationMatrixPlot.cpp +2/-2     
RimGenericJob.cpp +1/-0     
RimJobCollection.cpp +1/-1     
RimOpmFlowJob.cpp +8/-0     
RimOpmFlowJob.h +1/-0     
RimGridCalculation.cpp +3/-3     
RimWellTargetMapping.cpp +1/-1     
RimSummaryEnsemble.cpp +2/-2     
RimSummaryRegressionAnalysisCurve.cpp +3/-3     
RimWellLogRftCurve.cpp +2/-2     
CMakeLists_files.cmake +2/-0     
CMakeLists_files.cmake +1/-0     
RigContourMapProjection.cpp +3/-3     
RigFaultDistanceResultCalculator.cpp +4/-4     
RigIndexIjkResultCalculator.cpp +2/-2     
RigWellTargetMapping.cpp +8/-8     
CMakeLists.txt +2/-2     
CMakeLists.txt +2/-1     
RigEclipseCaseDataTools-Test.cpp +3/-1     
RigStatisticsTools-Test.cpp +6/-6     
TamComboBox.cpp +4/-1     
CMakeLists.txt +1/-1     

magnesj and others added 16 commits October 9, 2025 15:08
Ensures that active cell indexing is always used for temporary local grid refinements (LGRs) of radial multi-grids.

The  TestCase_Completions_Export loads a grid with several LGRs. This test case failed due to the changes for radial grids.
Connects the focus out event of the combo box to a slot that sets the current text to the field.
This ensures that the entered text is saved when the combo box loses focus.
Always call setSystemParameters after appendFishbonesSubs
Renames RiaStatisticsTools to RigStatisticsTools and moves it to the ResultStatisticsCache.
This change provides better organization and context for the statistics tools, as they are primarily used for result statistics caching.

Updates all references to the class to reflect the renaming and relocation.
Removes the redundant RiaStatisticsTools.h and RiaStatisticsTools.cpp from Application/Tools.
@qodo-code-review
Copy link

qodo-code-review bot commented Oct 14, 2025

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
No custom compliance provided

Follow the guide to enable custom compliance check.

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Avoid adding dependency on ApplicationLibCode

The RigGeoMechDataModel library should not depend on ApplicationLibCode. This
change introduces a problematic dependency cycle and violates clean
architectural principles.

Examples:

ApplicationLibCode/GeoMech/GeoMechDataModel/CMakeLists.txt [117]
                             CommonCode ResultStatisticsCache ApplicationLibCode

Solution Walkthrough:

Before:

# In ApplicationLibCode/GeoMech/GeoMechDataModel/CMakeLists.txt
target_link_libraries(
  RigGeoMechDataModel PUBLIC LibCore cafPdmCvf cafTensor cafUserInterface
                             CommonCode ResultStatisticsCache
)

After:

# In ApplicationLibCode/GeoMech/GeoMechDataModel/CMakeLists.txt
target_link_libraries(
  RigGeoMechDataModel PUBLIC LibCore cafPdmCvf cafTensor cafUserInterface
                             CommonCode ResultStatisticsCache ApplicationLibCode
)
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a significant architectural issue where a data model library (RigGeoMechDataModel) adds a dependency on a higher-level application library (ApplicationLibCode), which can lead to maintenance and dependency problems.

High
  • More

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants