investigate potentially problematic updating of well status from action handler

[steink]

As discussed in #6587, shut wells opened through actions will wrongly get a prevState status OPEN when entering the initialization phase in the next time-step. This leads to initialization problems (circumvented by #6587).

Discussed with @bska whether it also could have other implications. Reason for the problem is the ws.updateStatus() in BlackoilWellModelGeneric<>::updateEclWellsConstraints(). It's probably there for a reason, so just running through the tests to see if anything is triggered by commenting it out.

[steink]

jenkins build this failure_report please

[steink]

The commented line seems to be needed currently for shut-instructions during actions to function properly

[GitPaean]

What does the compareSeparateECLFiles do? The failures are not plotted.

mpi.compareSeparateECLFiles_flow+actionx_wlist
mpi.compareSeparateECLFiles_flow+actionx_wlist_4_procs

[steink]

What does the compareSeparateECLFiles do? The failures are not plotted.

I haven't looked at these in detail, but appears they compare results from two decks that only differ in actionx/udq setup, but that should result in the same logic (but doesn't with this change).

[GitPaean]

What does the compareSeparateECLFiles do? The failures are not plotted.

I haven't looked at these in detail, but appears they compare results from two decks that only differ in actionx/udq setup, but that should result in the same logic (but doesn't with this change).

Thanks for the response. Maybe those should also be in the failure_report to help investigation? @akva2

[akva2]

i guess we can but there is no ground truth so there is no telling what is good and bad data from the plots.

[GitPaean]

i guess we can but there is no ground truth so there is no telling what is good and bad data from the plots.

yes, you are right. I think those tests want two runs generate the similar enough results. Plotting can show the difference more directly, especially the well behavoirs.

[akva2]

yes i know that, but my point is you can't tell from the plots which of the simulation(s) was changed by your changes. the only thing we know is that they are supposed to be identical, and we already detect that they are not. i can add it, i just think its usefulness is limited.

[akva2]

in fact, in this case it shows absolutely nothing. results are identical, timestepping differs, https://ci.opm-project.org/job/opm-simulators-PR-builder/9004/

[GitPaean]

in fact, in this case it shows absolutely nothing. results are identical, timestepping differs, https://ci.opm-project.org/job/opm-simulators-PR-builder/9004/

from the curves, time stepping should also be the same?

[GitPaean]

in fact, in this case it shows absolutely nothing. results are identical, timestepping differs, https://ci.opm-project.org/job/opm-simulators-PR-builder/9004/

from the curves, time stepping should also be the same?

Keyword CTFAC:WI1:11,6,1 summary vector of different length (21 != 17)

But
https://ci.opm-project.org/job/opm-simulators-PR-builder/9004/artifact/mpi/build-opm-simulators/failure_report/05_actionx_wlist_4_procs.pdf

both curves have 21 time steps?

I might see things wrong.

[akva2]

no, time stepping is not the same. you can see it from the error output, one file uses 21 timesteps, one uses 17.

[GitPaean]

no, time stepping is not the same. you can see it from the error output, one file uses 21 timesteps, one uses 17.

yes, but the question is that no curves in the plotting use 17 time steps. both have more than 20 time steps. Excuse me if I read the plot wrong.

https://ci.opm-project.org/job/opm-simulators-PR-builder/9004/artifact/mpi/build-opm-simulators/failure_report/05_actionx_wlist_4_procs.pdf

[akva2]

correct you are, it did use the same results for both curves. should be ok now.

[akva2]

more useful now i guess, https://ci.opm-project.org/job/opm-simulators-PR-builder/9008/

[GitPaean]

more useful now i guess, https://ci.opm-project.org/job/opm-simulators-PR-builder/9008/

Yes. Actually showing well curves are identical/similar is also something helpful, because there is no obvious behavoir difference except rounding errors.

[akva2]

jenkins build this opm-common=4832 failure_report please

[akva2]

jenkins build this opm-common=4832 failure_report please

[GitPaean]

jenkins build this failure_report please

[GitPaean]

I realized this is rather complicated.

        ws.updateStatus(well.getStatus());
        ws.update_type_and_targets(well, st);

through applyActions().

It only updates the well status and re-initialize a new well state. There might not be much value to copy from it in WellState::init(), and also the rough values initialized might be problematic to use in a simulation.

[GitPaean]

I think now the key issue is that with the following code,

        ws.updateStatus(well.getStatus());
        ws.update_type_and_targets(well, st);

when called through applyActions, we need to initialize a better state, including the segments related.

Will try to come up with a suggestion. We might need to refactor the function WellState::initWellStateMSWell() and maybe other related initialization functions so it can handle one single well, while change mind in the process.