Log local switching #6145
Log local switching #6145
Conversation
|
jenkins build this please |
| const int iterationIdx = simulator.model().newtonMethod().numIterations(); | ||
| const int episodeIdx = simulator.episodeIndex(); | ||
| const int nupcol = schedule[episodeIdx].nupcol(); | ||
| if ( (oscillating && iterationIdx > nupcol)|| this->wellUnderZeroRateTarget(simulator, well_state, deferred_logger) || !(this->well_ecl_.getStatus() == WellStatus::OPEN)) { |
There was a problem hiding this comment.
In updateWellControl(..) it is iterationIdx >nupcol. This makes it consistent. But I don't know if waiting one iteration is on purpose or not.
There was a problem hiding this comment.
You mean what was introduced in #5753? Since we start counting iterations from 0 I believe we should use >= unless there is a clear (and documented) reason to do so.
There was a problem hiding this comment.
Since I made #5773, I assume > is a bug, I will update both instances to >= and retest.
There was a problem hiding this comment.
Thanks. Can be discussed if it is a bug, but prior to #6100 it was a mix between >= and >, and better to be consistent. Apparently I missed the one in updateWellControls in that PR..
totto82
force-pushed
the
log_local_switching
branch
from
4b1c6a2 to
37634a2
Compare
|
jenkins build this failure_report please |
totto82
force-pushed
the
log_local_switching
branch
from
37634a2 to
34075be
Compare
|
jenkins build this failure_report please |
|
I don't think the logging issue fixed in this PR is severe enough to delay the release, so I removed the release flag. |
|
jenkins build this failure_report please |
The log is used for output and checking for oscillations only count the swithcing after NUPCOL
totto82
force-pushed
the
log_local_switching
branch
from
34075be to
dc2f963
Compare
|
jenkins build this failure_report please |
|
It looks like mpi.compareECLFiles_flow+WVFPEXP-02 needs some attention and investigation.
|
OP-1 switches to GRUP in master the 1 Jly 2025. This is due to oscillation in the master and looks wrong.
. |
Sure. I read the result wrong and thought the blue line is the new results somehow. I will look at the PR later today. |
In the master, we don't log control switching during local solves, which could lead to changing controls without or with the wrong output in the terminal. The log also detects oscillations, and controls may oscillate significantly during the local solves. I added a safeguard to restrict only changing controls for iter>nupcol and the local solves, but we may need a more robust solution. I tagged this with release since it is a bug fix, but I also made it a draft as it requires more testing.
UPDATE: After some thinking and testing, I suggest starting to count oscillations only after NUPCOL iterations. After NUPCOL iterations, the control target is fixed, and we should, in principle, experience fewer oscillations. With this change, we less frequently end up with the "wrong" control, while the "cost" of stagnating simulations due to infinite oscillations is still avoided. The current issue in master that we don't log control changes during local switching (leading to missing log information to terminal and .PRT) is still fixed in the PR.