opm-simulators
opm-simulators copied to clipboard
OPM
fixed : make (MSW|Std) well copyable again
the reference member in the base classes would point to the original instance, not the new instance
I think this is an important bugfix, so we should understand why the test cases fail. I am a bit surprised, I would expect only cases that include some dependency on well potentials (i.e. having group controlled wells) to be affected! @steink @GitPaean or @totto82 maybe one of you can aid @akva2 investigating?
I would expect only cases that include some dependency on well potentials (i.e. having group controlled wells) to be affected!
Most of the test failures are due to small changes in the output of well potentials. I haven't systematically looked at all cases, but the once I have looked at seems reasonable.
Most of the test failures are due to small changes in the output of well potentials. I haven't systematically looked at all cases, but the once I have looked at seems reasonable.
Then it is not so strange! I would expect the results to be more correct, do you see any cases with significant change to the potential output?
With the current bug, even adding output of well potentials (triggering their calculation where it was not done before) might change the simulation results if it mutates the incorrectly copied base object, I do not know if this actually happens though.
Most of the test failures are due to small changes in the output of well potentials. I haven't systematically looked at all cases, but the once I have looked at seems reasonable.
Small changes in the well potential looks like affecting mostly group control a little bit, not other cases, maybe also well testing. Most of the tests do not look worrying at all. We just need to go through the tests quickly to avoid surprises.
I have the same impression, I haven't seen anything worrying. I am a bit surprised that it affects this many cases though, but there's probably some effect I haven't thought of. I haven't testet, but with this, I think we will see less convergence failures for potential calculations for stopped wells, since in that case (in master) convergence tollerances for the (bhp/thp) control eq will be of type rate.
So probably no worries here, but I guess we should make sure we don't miss anything. @GitPaean and @totto82 , could we just devide the 60 failed tests between us to confirm (Kai 1-20, Tor Harald 21-40, Stein 41-60)?
So probably no worries here, but I guess we should make sure we don't miss anything. @GitPaean and @totto82 , could we just devide the 60 failed tests between us to confirm (Kai 1-20, Tor Harald 21-40, Stein 41-60)?
How about let me go through all of them and report back. Might delegate you guys with ones need more detailed investigation.
Probably tomorrow.
As promised, reporting back the checking of the regression failures.
Will provide some information regarding the ones need more detailed investigations.
The following three cases are the ones need a little more detailed checking. (black line is the master result, green line is the PR result)
mpi.compareECLFiles_flow+0A4_GRCTRL_LRAT_LRAT_GGR_BASE_MODEL2_STW
mpi.compareECLFiles_flow+UDQ_WCONPROD
mpi.compareECLFiles_flow+0_BASE_MODEL2_LET
Checking regression failures with big scale is quite expensive. It is good if we can have some more automatic way to help.
And also, because it is quite some efforts to check one by one, and some update of the reference might be done without careful checking from time to time. With time, the reference changes relatively significantly without noticing as we found sometimes.
Looks like there are some issues that needs fixing here. Seems to be missing a calculateExplicitQuantities in the potential calculations for STW. Hence, with the PR, there is zero p-drop along the well for the well_copy. In master it's non-zero since this->connections_.pressure_diff(perf) corresponds to the p-drop of the original well.
Looks like there are some issues that needs fixing here. Seems to be missing a
calculateExplicitQuantities()in the potential calculations for STW. Hence, with the PR, there is zero p-drop along the well for thewell_copy. In master it's non-zero sincethis->connections_.pressure_diff(perf)corresponds to the p-drop of the original well.
Thanks for looking into this. It look reasonable since the majority of the failing tests (if not tall) are related to STW.