OPM
fixed: do not dereference invalid iterator
if a well has connections added in an action, the well perf data is not yet initialized in the call to updateEclWells. it will however be reinitialized before the next time step so simply adding a bounds check here avoid the invalid dereference and the connection factor will take effect through the reinitialization.
ref e.g. https://ci.opm-project.org/job/opm-simulators-static-analysis/383/testReport/junit/(root)/debug_iterator/compareSeparateECLFiles_flow_pyaction_gruptree_insert_kw/
You mean the perf data will be initialized here at the beginning of each time step (https://github.com/OPM/opm-simulators/blob/master/opm/simulators/wells/BlackoilWellModel_impl.hpp#L436 and then https://github.com/OPM/opm-simulators/blob/master/opm/simulators/wells/BlackoilWellModel_impl.hpp#L343) , right?
Looks fine :) @akva2: can you possibly add a comment about the newly introduced check and why it is needed?
yes precisely, the bool wellStructureChangedDynamically be set to true in the call to updateEclWells() thus triggering that block of code. Have added a comment.
I've looked into this more closely now and as far as I can tell, the proposed change here only papers over a deeper problem and doesn't really fix anything other than ensuring we don't dereference an end() iterator. While mostly benign, I think I'd prefer that we fix the real problem instead of merging this.
Maybe share what the real problem is?
Sure. The real problem is that if there are dynamic changes to the a well's connections within an action block–e.g., if there are new connections introduced in COMPDAT or some connections open and/or some connections close through WELOPEN, then the data structure used in updateWellsEcl() is inconsistent and the invariants upon which the current logic depends do not hold. Function updateWellsEcl() was created (PR #3116, commit 98f6a9a7a) to solve a much simpler problem than we're currently using it to solve–opening/closing a well in response to keywords like WELOPEN in an ACTIONX block. The connection loop that's altered here was introduced as a copy of the WELPI handling that I added in PR #2866 (commit 319c24033).
The problem is that it's just not safe to run that loop at all if there are new connections being added and/or connections being opened/closed. A quick workaround that's probably safe in most cases is to completely remove that loop here and to make sure that we record a structural change in the dynamic WELPI handler, but the real solution will require reworking our machinery for imparting dynamic well or group changes from the ACTIONX handler to the simulator layer. A list of "affected wells" is not sufficient anymore.