Skip to content

FlowProblem: drop a stray cell-index assignment in updateMaxWaterSaturation_ - #7296

Open
hnil wants to merge 1 commit into
OPM:masterfrom
hnil:pr/maxwatsat-cell-index-bug
Open

FlowProblem: drop a stray cell-index assignment in updateMaxWaterSaturation_#7296
hnil wants to merge 1 commit into
OPM:masterfrom
hnil:pr/maxwatsat-cell-index-bug

Conversation

@hnil

@hnil hnil commented Aug 7, 2026

Copy link
Copy Markdown
Member

maxWaterSaturation_ is a std::vector<Scalar> sized by grid cells and indexed by element index everywhere it is used (FlowProblem.hpp:1170, :1395, :1594, :1845, FlowGenericProblem_impl.hpp:621). It has no time dimension, so

this->maxWaterSaturation_[/*timeIdx=*/1] = this->maxWaterSaturation_[/*timeIdx=*/0];

does not shift a time level — it overwrites cell 1's running maximum with cell 0's. The per-cell update that follows only ever raises values, so cell 1 keeps the wrong maximum whenever cell 0 is the larger of the two.

updateMinPressure_() is the structural twin on the same ROCKCOMP path and has no such line.

Affects ROCKCOMP runs only, and only cell 1. SPE1CASE2_ROCK2DTR is byte-identical with and without the change, because cells 0 and 1 reach the same maximum there — so it is latent on the decks in the test suite.

Found while auditing #7198: this line is the sole reason that rolling maxWaterSaturation_ back across a failed timestep changes results. Every other quantity in that snapshot is a plain monotone extremum recomputed from the restored solution, hence idempotent; this one is not, purely because of this assignment.

…ration_

maxWaterSaturation_ is a std::vector<Scalar> sized by the number of grid
cells and indexed by element index everywhere it is used.  It has no
time dimension, so

    maxWaterSaturation_[/*timeIdx=*/1] = maxWaterSaturation_[/*timeIdx=*/0];

does not shift a time level -- it overwrites cell 1's running maximum
with cell 0's.  Since the per-cell update that follows only ever raises
the value, cell 1 keeps the wrong maximum whenever cell 0 is the larger
of the two.

The structurally identical updateMinPressure_(), which serves the same
ROCKCOMP path, has no such line.

Only affects ROCKCOMP runs, and only cell 1.  SPE1CASE2_ROCK2DTR is
byte-identical with and without this change, because cells 0 and 1 reach
the same maximum there.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hnil hnil added the manual:bugfix This PR is a bug fix and should be noted in the manual label Aug 7, 2026
@hnil
hnil requested a review from akva2 August 7, 2026 16:35
@hnil
hnil requested a review from bska August 7, 2026 16:40
@bska

bska commented Aug 7, 2026

Copy link
Copy Markdown
Member

jenkins build this please

@bska

bska commented Aug 7, 2026

Copy link
Copy Markdown
Member

jenkins build this failure_report please

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

Labels

manual:bugfix This PR is a bug fix and should be noted in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants