[test] different approach - #7337
Draft
GitPaean wants to merge 6 commits into
Draft
Conversation
GitPaean
added
the
manual:irrelevant
Member
Author
|
jenkins build this failure_report please |
Member
Author
|
jenkins build this failure_report please https://ci.opm-project.org/job/opm-simulators-PR-builder/10467/ |
Member
Author
|
The regression for this case is the same with OPM/opm-common#5298 and #7333 for the same reason. |
GitPaean
force-pushed
the
network-thp-fixes
branch
3 times, most recently
from
August 21, 2026 10:31
5cef09c to
1e904ac
Compare
Member
Author
|
jenkins build this failure_report please |
GitPaean
force-pushed
the
network-thp-fixes
branch
from
August 21, 2026 12:27
1e904ac to
b3bbf42
Compare
Member
Author
|
jenkins build this failure_report please |
Contributor
There was a problem hiding this comment.
Pull request overview
Refines well-control handling and preserves network-imposed THP limits across network detachment and state restoration.
Changes:
- Removes redundant trivial-group-target state.
- Prevents network THP control without a VFP table.
- Tracks and restores network-imposed THP limits.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
WellInterface_impl.hpp |
Removes trivial-target bookkeeping. |
WellConstraints.cpp |
Relies on zero-rate early returns. |
SingleWellState.hpp |
Removes obsolete serialized state. |
SingleWellState.cpp |
Removes initialization and equality handling. |
BlackoilWellModelNetworkGeneric.hpp |
Adds persistent THP-limit maps. |
BlackoilWellModelNetworkGeneric.cpp |
Applies, retains, and clears network THP limits. |
Suppressed comments (1)
opm/simulators/wells/BlackoilWellModelNetworkGeneric.hpp:148
- This new persistent state is not reconstructed by the file-restart path:
setFromRestart()restores only node pressures, andassignNodeAndBranchValues()emits values only for wells currently belonging to network-node groups. Therefore, after restarting a case where a well was already detached, this map is empty and the retained THP limit is lost, changing the continuation result. Persist or reconstruct the per-well imposed limits in the ECL restart path.
// The latest THP limit the network imposed on each well. A well that is
// detached from the network keeps its network-imposed THP limit until new
// production controls are specified for it (matching Eclipse behavior).
std::map<std::string, Scalar> network_imposed_thp_limits_;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
GitPaean
force-pushed
the
network-thp-fixes
branch
from
August 22, 2026 21:31
b3bbf42 to
18e245a
Compare
Member
Author
|
jenkins build this failure_report please |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
opm/simulators/wells/BlackoilWellModelNetworkGeneric.cpp:359
- The added test only verifies that these maps survive serialization. None of the behavioral branches introduced here are covered: retaining a limit after detachment, clearing it on a production update (including ACTIONX), restoring it after a failed step, and skipping wells without VFP tables. Please add focused regression coverage for these state transitions; otherwise failures such as clearing the limit for unrelated ACTIONX well updates remain undetected.
well.setDynamicThpLimit(it->second);
network_imposed_thp_limits_[well.name()] = it->second;
Member
Author
|
jenkins build this failure_report please |
A production well attached to an extended-network node but with a defaulted VFP table cannot translate the nodal pressure into a BHP limit. Imposing the dynamic THP limit made wellHasTHPConstraints() return true and the following VFP evaluation aborted the run with "Nonexistent VFP table 0 referenced". Matching Eclipse, such wells now stay on their rate controls while their rates still contribute to the network flows and the nodal pressure. Also keep the last network-imposed THP limit in force for wells detached from the network, e.g. moved to a non-network group with WELSPECS, until new production controls are specified for the well. The limits are stored in BlackoilWellModelNetworkGeneric and dropped on a PRODUCTION_UPDATE event.
activeProductionConstraint() skipped the switch to THP control for GRUP-controlled wells when ws.trivial_group_target was set. That flag is only rewritten in updateWellStateWithTarget() when a well switches control, so it can go stale - it is set at the initial ORAT->GRUP switch while a sibling well still covers the whole group target - and the veto then blocks the very switch that would refresh it. Network wells could therefore keep flowing with a THP below the nodal pressure of the node they feed, which Eclipse never does. Both call paths into checkIndividualConstraints() already return early for wells whose freshly evaluated group target is zero (stoppedOrZeroRateTarget() and wellUnderZeroRateTarget()), so the veto could only ever act on stale information and is removed.
The preceding commit removed the only reader of this flag, leaving write-only state that was still computed, serialized and compared. Wells with a trivial group rate target are recognised on demand by stoppedOrZeroRateTarget() and wellUnderZeroRateTarget(). The else branch of the rate scaling only existed to set the flag and goes with it; the scaling itself is unchanged.
Production controls specified through ACTIONX are applied to the affected wells by updateEclWellsConstraints() without waiting for the next report step. Forget any THP limit the network has imposed on such a well and reset the dynamic THP limit of the live well object there as well, so that a well detached from the network does not keep a stale network-imposed THP limit after new production controls are specified. The entries are also erased from the last-valid state since schedule changes survive retried time steps. A well that is still attached to a network node receives a fresh limit at the next network balance.
Populate the current and last-valid network-imposed THP limit maps with distinct entries in the serialization test fixture, so that omitting or reordering either field in the serialization would be detected by the round-trip test.
An action can update a well without changing its production controls, for instance a pure WELOPEN. Gate the invalidation of the network-imposed THP limit on a PRODUCTION_UPDATE event being recorded for the well at the current report step - the same criterion used at report step initialization - so that updates which leave the production controls untouched keep the retained limit of a well detached from the network.
GitPaean
force-pushed
the
network-thp-fixes
branch
from
August 25, 2026 13:09
5a560b0 to
891617e
Compare
Member
Author
|
jenkins build this failure_report please |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.