Add CONVERGENCE as closing reason #4529
Conversation
|
jenkins build this opm-simulators=6067 please |
|
jenkins build this opm-simulators=6097 please |
1 similar comment
|
jenkins build this opm-simulators=6097 please |
|
The WGRUPCON failures is fixed with |
|
jenkins build this opm-simulators=6097 please |
| @@ -32,6 +32,7 @@ | |||
| #include <string> | |||
| #include <unordered_set> | |||
| #include <vector> | |||
| #include "WellTestState.hpp" | |||
There was a problem hiding this comment.
We already include that header at the top of the file.
| @@ -60,6 +60,7 @@ enum class Reason { | |||
| GROUP = 4, | |||
| THP_DESIGN=8, | |||
| COMPLETION=16, | |||
| CONVERGENCE=32, | |||
There was a problem hiding this comment.
I suppose this is fine, but you need to be aware that we'll throw an exception, and not create any restart file output. if the well is closed due to CONVERGENCE at restart file writing time.
There was a problem hiding this comment.
That's not good. Can we return WTest::EclCloseReason::NONE for CONVERGENCE to avoid adding new reasons to the restart files? Otherwise, we may break restart compatibility. The force-shut option for non-convergent wells is a back-door solution when we cannot converge the well and properly shut it down due to operability. Currently, they are tagged as PHYSICAL. But since the purpose of these PRs is to allow for checking these wells regardless of WTEST I think setting them as NONE is more robust.
There was a problem hiding this comment.
Do we ever shut wells due to WellTestConfig::Reason::NONE?
There was a problem hiding this comment.
Do we ever shut wells due to
WellTestConfig::Reason::NONE?
I don't think so, but you know the well testing module better than I. That said, it would be a little strange for a closed/stopped well in the restart file to have a NONE reason. Such a well, at restart time, would be treated as open as far as the well testing code is concerned.
Is that okay?
There was a problem hiding this comment.
I think so. It is better to open this well than to potentially shut it forever (like we do now). It will break perfect restart, but I think that is better than breaking restart compatibility.
totto82
force-pushed
the
addConvergenceReason
branch
from
e988c57 to
8621458
Compare
|
jenkins build this opm-simulators=6097 please |
|
jenkins build this opm-simulators=6097 failure_report please |
|
Yes I know, but I haven't had time to look at it. I don't think this will be ready for release. I am not happy with the way we treat "failed wells" in master, so we should keep up the discussion. |
Motivation is to make it easy always to try to reopen wells that are shut due to convergence.
See OPM/opm-simulators#6067