Changes in how the simulator handles NUPCOL #5724
Conversation
|
jenkins build this please |
1 similar comment
|
jenkins build this please |
|
jenkins build this failure_report please |
|
I'm currently looking into this, but having some unrelated issues with recent updates, so not done testing |
| // We also dont allow wells to change to group controls | ||
| // when iterationIdx > nupcol |
There was a problem hiding this comment.
Should the opposite also apply, e.g., that wells under group control should not be allowed to change to individual in the subsequent updateWellControl (individual mode)?
There was a problem hiding this comment.
This change indeed reduce oscillations and improve convergence, but it has the potential to give very wrong results. I think we rather should remove the if statement completely i.e. always check the constrains. This will need some adjustment in the code to handle oscillatory controls for instance when a higher group has a constraint that is equal to the sum of the sub groups controls. I am testing out an option of detecting oscillatory controls like we do for the wells. This seems to work but need more testing. As a first move I plan to remove this part of this PR to be able to evaluate and test these things separately.
…ative tolerance. Default is 0.001
totto82
force-pushed
the
changeNUPCOL
branch
from
04d4364
to
d7857b4
Compare
|
jenkins build this failure_report please |
1 similar comment
|
jenkins build this failure_report please |
| "at iteration {}. Update {} for Group {} even if iteration is larger than {} given by NUPCOL." , | ||
| tol_nupcol, iterationIdx, control_str, gr_name, nupcol); | ||
| deferred_logger.info(msg); | ||
| } |
There was a problem hiding this comment.
This should be changes to .debug before merging.
totto82
force-pushed
the
changeNUPCOL
branch
from
d7857b4
to
554acb4
Compare
|
jenkins build this failure_report please |
| phaseIdx, | ||
| /*isInjector*/ false); | ||
| } | ||
| Scalar rel_change = std::abs( (gr_rate_nupcol - gr_rate) / (0.5*gr_rate_nupcol + 0.5*gr_rate)); |
There was a problem hiding this comment.
Is there a possibility both group rates in denominator could be zero here? Or would that situation lead to failure at an earlier stage?
There was a problem hiding this comment.
It is possible so I will add a check. Maybe we should have a "large epsilon" here since relative changes does not make much sense for very small numbers.
|
I'm running a tests on a larger model, and will report back. Otherwise PR is now simpler and tests seem fine except |
|
Larger testing looks fine on my side. Did some investigation on |
|
jenkins build this failure_report please |
Does it make sense to not allow re-opening wells when iter > nupcol? This could reduce oscillations in the VREP control, but also to premature closing of wells. Often the closing/opening of individual wells stabilize after some iterations so maybe worth a try to test out? |
|
When I only allow for well opening when iter < nupcol I get back the reference solution. I will do some more tests with this and report back.
|
I agree that it should not be allowed to re-open at some stage, but not sure whether this should be related to nupcol (the problem is not neccessarlily related to groups). |
I see your point. It seems to stabilize some corner cases thought. Does this mean that we accept the results of well C-1H in 02_WGRUPCON and can start the process of getting this PR in? |
It's OK from my side. |
TODO