-
Notifications
You must be signed in to change notification settings - Fork 117
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
Update Well Topology if Triggered From ACTIONX #4749
Update Well Topology if Triggered From ACTIONX #4749
Conversation
As an example of the effect, the figure below compares the field pressure ( --solver-max-time-step-in-days=1.0 using the current master sources ( |
jenkins build this please |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me (assuming that we catch all those structure changes), I would have merged this as it seems to fit urgent issues, but it is marked as WIP and I assume that there is a reason for that.
ecf4284
to
5cbada0
Compare
d545349
to
b6124c3
Compare
1d67506
to
3320ce6
Compare
3320ce6
to
79c21a5
Compare
f562b1a
to
84c9a03
Compare
jenkins build this please |
6c0f9c3
to
6bc7ddd
Compare
Thanks for the clarification. This now triggered a thought on my side: I have previously been wondering why this works in parallel when it seemed that all code works only on the local process without any communication. I concluded that is probably OK because the state that the ACTIONX evaluates is shared between the processes (and I assumed that the wells triggering the action are a superset of the affected wells). Now this is not really the case, when an action gets triggered for a set wells but it affects another set of wells. In a parallel run the set of affected wells might only contain wells that do not perforate local wells while the triggering set might have wells which perforate local cells in it, and vice versa. Could it be that in parallel we correctly evaluate the actions for local wells, but if the affected well is on another process (no perforated cell on this process) then we do nothing? Maybe this did not happen before because at report steps we do the necessary computation implicitly? |
I believe that the current action handling logic is correct, even in parallel, although there may be issues if the run uses distributed wells. I haven't analysed that case completely. My reasoning is as follows
All in all I believe that this means each rank computes the correct set of mathcing and affected wells for each action, but only mutates the local wells. |
93b9896
to
03210d4
Compare
jenkins build this please |
ceeaf06
to
4ed9d9e
Compare
This commit adds a new flag data member, wellStructureChangedDynamically_ to the generic black-oil well model. This flag captures the well_structure_changed value from the 'SimulatorUpdate' structure in the updateEclWells() member function. Then, in BlackoilWellModel::beginTimeStep(), we key a well structure update off this flag when set. This, in turn, enables creating or opening wells as a result of an ACTIONX block updating the structure in the middle of a report step.
Needed for increased determinism in sensitive test (WBHP trigger).
4ed9d9e
to
a2fa381
Compare
jenkins build this update_data please |
Reason: PR OPM/opm-simulators#4749 opm-common = 980ac2599acf475129019e8de8a352b05d8b280d opm-grid = a165a07dcc65ab6af628e8e1d21e1bbef83cca8a opm-models = 818c4b97056b49f40346c070a25ee2f682f00cb5 opm-simulators = db3d15ec2ce66418ab4b932e16161da0b1b08359 ### Changed Tests ### * actionx_m1
jenkins build this opm-tests=1085 please |
Automatic Reference Data Update for PR OPM/opm-simulators#4749
The new reference solutions have been installed on the CI system. Merging per prior approval. |
@bska In retrospect, I have questions about this (because I have similar problems): The reason for the problem (changes occur to the wells only on the next report step) was, that these changes happen(ed) in the simulator due to Events signaled by the schedule. This happens at the next report step (because that is where the schedule is changed and that is where we query for events). Probably not a real problem here, because it is just additional work, but does not change anything, but for other keywords (e.g. NEXTSTEP) it might be. |
This commit adds a new flag data member,
bool wellStructureChangedDynamically_
to the generic black-oil well model. This flag captures the
value from the
SimulatorUpdate
structure in theupdateEclWells()
member function. Then, inBlackoilWellModel::beginTimeStep()
, we key a well structure update off this flag when set. This, in turn, enables creating or opening wells as a result of anACTIONX
block updating the structure in the middle of a report step.