-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set option maximum-number-of-well-switches default value to 10 #5599
base: master
Are you sure you want to change the base?
Conversation
jenkins build this please |
(just in case: you need to include failure_report in the trigger for the pdfs to be generated) |
Argh, yes forgot |
jenkins build this failure_report please |
build will fail because #5598 was not yet merged. |
OK, so build with |
no can't have two pr's for opm-simulators... have to wait for it to be merged. |
which is now done. |
jenkins build this failure_report please |
It looks like when we click on view, we are opening the raw pdf file? which is not very helpful. I would suggest when we click on |
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. |
I figured that out, but since the |
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. |
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. |
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. |
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. |
The number 10 was a bit arbitrary from our side too. From our testing we could just as well go with 5. |
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? |
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. |
16ad4ee
to
3cdb749
Compare
Reconsidering this as #5641 has issues that may not be resolved before release. |
jenkins build this failure_report please |
jenkins build this failure_report please |
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).