OPM / opm-simulators

Simulator programs and utilities for automatic differentiation.
http://www.opm-project.org
GNU General Public License v3.0
111 stars 122 forks source link

Set option maximum-number-of-well-switches default value to 10 #5599

Open steink opened 2 weeks ago

steink commented 2 weeks ago

Increasing default value of option --maximum-number-of-well-switches from 3 to 10. Fixes a few problematic cases and otherwise do not seem to do any harm (performance-wise). Will break many tests, though (will go through them).

steink commented 2 weeks ago

jenkins build this please

akva2 commented 2 weeks ago

(just in case: you need to include failure_report in the trigger for the pdfs to be generated)

steink commented 2 weeks ago

(just in case: you need to include failure_report in the trigger for the pdfs to be generated)

Argh, yes forgot

steink commented 2 weeks ago

jenkins build this failure_report please

akva2 commented 2 weeks ago

build will fail because https://github.com/OPM/opm-simulators/pull/5598 was not yet merged.

steink commented 2 weeks ago

build will fail because #5598 was not yet merged.

OK, so build with opm-simulators=5598?

akva2 commented 2 weeks ago

no can't have two pr's for opm-simulators... have to wait for it to be merged.

akva2 commented 2 weeks ago

which is now done.

akva2 commented 2 weeks ago

jenkins build this failure_report please

GitPaean commented 2 weeks ago

It looks like when we click on view, we are opening the raw pdf file? which is not very helpful.

image

https://ci.opm-project.org/job/opm-simulators-PR-builder/6624/artifact/mpi/build-opm-simulators/failure_report/01_udq_pyaction.pdf/*view*/

I would suggest when we click on view, we say some output from the compareECL.

akva2 commented 2 weeks ago

don't click on view, click on the filename. view is only useful for ascii files. i cannot control that. you can see the compareECL output by looking at the output of the failed test. this is just how jenkins works, all i do is attach pdf artifacts.

GitPaean commented 2 weeks ago

don't click on view, click on the filename....

I figured that out, but since the view button is there...

GitPaean commented 2 weeks ago

don't click on view, click on the filename. view is only useful for ascii files. i cannot control that. you can see the compareECL output by looking at the output of the failed test. this is just how jenkins works, all i do is attach pdf artifacts.

Thanks for the explanation. I have no ideas about how the things on Jenkins work.

I see I can still see the compareECL output in the original place. A potential small improvements is that the pdf and the compareECL output can be in the same line. But I have no ideas what efforts it might cause.

But no worries, in its current form it is already a huge help for investigating jenkins regression, which is much appreciated.

akva2 commented 2 weeks ago

it would mean digging into the java code of jenkins and/or writing a new groovy or java based plugin for jenkins. it would be a rather involved effort.

totto82 commented 2 weeks ago

The maximum-number-of-well .. options was added to avoid shutting of wells some time ago. I think it is still needed for some cases to avoid issues. The downside is that the simulator now can accept solution where lot of the well/group constrains are broken. For instance thp << thp_limit. Some time ago I did some work to fix this more properly. See https://github.com/OPM/opm-simulators/pull/3410. Maybe it is time to resurrect that work. @steink Could you elaborate a bit on which models this fixes? Either here or by email.

steink commented 2 weeks ago

The maximum-number-of-well .. options was added to avoid shutting of wells some time ago. I think it is still needed for some cases to avoid issues. The downside is that the simulator now can accept solution where lot of the well/group constrains are broken. For instance thp << thp_limit. Some time ago I did some work to fix this more properly. See #3410. Maybe it is time to resurrect that work. @steink Could you elaborate a bit on which models this fixes? Either here or by email.

With the new defaults, we seem to be a bit more sensitive to this option. One example is the standard Norne test-model where one well causes a lot of problems because it is never able to switch to the correct control due to this limit (due to oscillations in surrounding reservoir pressure during Newton iterations). I'll send you some more background. I'll have a look at #3410 as this seems to be very relevant for a different but related problem I'm looking into.

totto82 commented 2 weeks ago

I can see if I am able to rebase #3410 on current master and make it ready again. The number 3 was a bit ad-hoc, so I am not against changing the default number. 10 will in essence turn it off since we only allow one switch per newton iteration and the maximum newton is typically <= 20.

alfbr commented 2 weeks ago

The number 10 was a bit arbitrary from our side too. From our testing we could just as well go with 5.

totto82 commented 2 weeks ago

Maybe a more robust logic here is to set it to the individual constrain when oscillating. i.e. for a well that oscillates between well and group control, we force to use the well control if oscillating more than X number of times.

steink commented 2 weeks ago

Maybe a more robust logic here is to set it to the individual constrain when oscillating. i.e. for a well that oscillates between well and group control, we force to use the well control if oscillating more than X number of times.

Good point. In addition, I think when oscillating between pressure (bhp or thp) and some rate one should force to use pressure control since a rate control might not be feasible which in turn may lead to convergence issues or problematic pressures. So if a well is oscillating between two controls, we force the control with highest priority according to e.g., 1. bhp, 2. thp, 3, various well rates, 4. group control?

totto82 commented 2 weeks ago

Good point. In addition, I think when oscillating between pressure (bhp or thp) and some rate one should force to use pressure control since a rate control might not be feasible which in turn may lead to convergence issues or problematic pressures. So if a well is oscillating between two controls, we force the control with highest priority according to e.g., 1. bhp, 2. thp, 3, various well rates, 4. group control?

I can test it out in the #5607 branch. For the GASLIFT-12 case I am working on the controls switch between ORAT and THP and is kept at ORAT which gives strange results. I can report back when I have tested it.