14345 grid calculation undefined values - #14348
Conversation
69c2c44 to
3b93eb4
Compare
3b93eb4 to
604d498
Compare
kriben
left a comment
kriben
left a comment
There was a problem hiding this comment.
Review bot working on behalf of Kristian Bendiksen.
The undefined-value and calculated-result handling changes look well motivated, and the new tests cover several important paths. I found two important correctness issues in the new data-filter evaluation: combined range filters use incorrect masks on non-target grids, and inactive range/combined filters are still applied. There is also an LGR issue when a combined filter contains property filters.
| cvf::UByteArray childMask( n ); | ||
| childMask.setAll( 1 ); | ||
| child->applyToCellVisibility( &childMask, grid, timeStepIndex ); | ||
| child->applyToCellVisibility( &childMask, grid, timeStepIndex, sourceCaseOverride ); |
There was a problem hiding this comment.
Important: An INCLUDE range child produces the wrong mask on grids other than its target grid. RimCellFilter::applyToCellVisibility() deliberately returns without changing the mask in that case, so this all-true childMask remains all true and the combined filter includes every cell on the non-target grid. Please initialize/evaluate range-child masks with the same target-grid semantics used by computeReservoirCellVisibility (INCLUDE contributes no cells; EXCLUDE removes none), including the intended parent-grid propagation.
| cvf::ref<cvf::UByteArray> | ||
| RimCellFilterTools::computeReservoirCellVisibility( RimCellFilter* filter, RimEclipseCase* eclipseCase, size_t timeStepIndex ) | ||
| { | ||
| if ( !filter || !eclipseCase || !eclipseCase->eclipseCaseData() || !eclipseCase->eclipseCaseData()->mainGrid() ) return nullptr; |
There was a problem hiding this comment.
Important: Please handle !filter->isFilterEnabled() before evaluating the selected data filter. Property filters currently no-op when unchecked, but base range/index filters and RimCombinedFilter do not check their own active state, so an unchecked range or combined filter still changes the calculation. This makes filter activation behave differently depending on filter type.
| // of a 3d view. Geometry based filters (range and index, e.g. an INDEX_K polygon) can fail to | ||
| // select the refined cells directly on a fine LGR, so propagate the parent grid visibility. | ||
| // Property filters evaluate each cell against its own result value and must be left untouched. | ||
| const bool isGeometryFilter = filter->isRangeFilter() || filter->isIndexFilter(); |
There was a problem hiding this comment.
Suggestion: RimCombinedFilter is declared with FilterDefinitionType::INDEX, so this treats every combined filter as geometry-only. If it contains a property child, the OR/AND with parentMask can override the LGR cell's own property evaluation—for example, a matching parent makes all refined children visible for an INCLUDE combined filter. Please make propagation depend on the combined filter's children, or perform geometry propagation at child evaluation time before combining masks.
604d498 to
1a0aa6c
Compare
1a0aa6c to
66b4fac
Compare
…in aggregations Aggregation functions like sum() in the expression parser include all values in the input vector. Undefined values (HUGE_VAL) could leak into the aggregation input when a grid case group member has fewer time steps on disk than the main case it inherits result meta data from, or when a cell is active in the calculation case but has no value in the source data. This produced infinite sums for some realizations, while the histogram statistics ignore undefined values. Replace undefined values in the aggregation input with the default value 0.0, matching the histogram semantics, and log a warning per case and variable when values were replaced.
…togram statistics Log at debug level, for each case and time step, how many input values contribute to a grid calculation aggregation and how many were replaced with the default value (non-visible cells and undefined values). Also log the sum and number of values used when computing visible cells statistics, to allow comparison with the histogram sum in the 3d view.
Add a sourceCaseOverride parameter to RimCellFilter::applyToCellVisibility. When set, property filters evaluate against the override case's result values instead of the case the filter is bound to, allowing the same filter definition to be evaluated against each case in an ensemble. Combined filters forward the override to their children, and geometry based filters ignore it. Add RimCellFilterTools::computeReservoirCellVisibility, which evaluates a cell filter against a given case for all grids and returns the visibility indexed by reservoir cell index. Range filters propagate the parent grid visibility into LGR cells, matching the filtered geometry of a 3d view. Also add RimCellFilter::setFilterMode.
…ters Add a data filter option to grid calculations. Data filters are evaluated per calculation case and time step, allowing the same filter definition to be applied across an ensemble while using each case's own result values. Keep the existing visible-cell semantics for the cell filter view option, including property filters evaluated in that view. Pass visibility masks directly to result filtering so both filter types share the downstream calculation path.
Add a data filter collection to RimReservoirGridEnsemble, so filters can be defined once for the ensemble instead of per realization. The filters are bound to the main case of the ensemble for configuration, and evaluated per case as elsewhere. The filter creation commands are available from a Data Filters submenu on the ensemble node, and the collection node is hidden while empty, matching the case-level behavior. The grid calculation data filter options include the filters of the ensemble the destination case belongs to.
The grid calculation data filter evaluated index filters (polygon, user defined) directly on every grid. An INDEX_K polygon evaluated on a fine LGR can select no cells, so refined cells whose parent host cell is inside the polygon were dropped from the aggregation. This gave a lower sum than the histogram of visible cells in the 3d view, which renders LGR cells following their parent grid cell. Hoist the parent grid visibility propagation out of the range filter branch so it also runs for index filters. Property filters are evaluated per cell against their own result values and are left untouched.
66b4fac to
01b34dc
Compare
1 participant
Fixes #11559.
Fixes #14345.