Reworked implementation of group control feasibility by steink · Pull Request #6963 · OPM/opm-simulators

steink · 2026-03-24T14:12:18Z

Rewritten #6889 with better functionality and hopefully less impact on cases where it should not be relevant. Main differences from #6889:

Added group production/injection mode to well-state group_target
Added additional group_target_fallback to well-state
Added flag use_group_target_fallback to well-state
Now both getGroupProductionTargetRate and getGroupProductionControl selects group-target based on above flag
Flag is updated before each local linear solve (to prevent singularities) and in updateWellStateWithTarget (to prevent bad scaling)
Flag is set to true if the ratio well-guiderate/group-guiderate or well (top segment) fraction of current group control mode is less than group_control_fraction_tolerance
Added parameter group_control_fraction_tolerance which is currently defaulted to 1e-4.

Will do some more testing before asking for review

steink · 2026-03-24T14:13:35Z

jenkins build this failure_report please

steink · 2026-03-27T09:20:08Z

jenkins build this failure_report please

steink · 2026-03-27T09:55:26Z

I'll make a relevant test since none of the current tests are influenced by this change.

GitPaean · 2026-03-27T10:01:00Z

Thank you for the request, I will look into this by Monday.

Copilot

Pull request overview

This PR reworks “group control feasibility” for producer wells by introducing a per-well group-target fallback and a runtime flag that switches which group target/mode is used when the current group control mode becomes numerically infeasible.

Changes:

Extend SingleWellState to store group_target control mode metadata plus group_target_fallback and use_group_target_fallback.
Add logic to compute and select fallback group targets/modes (including guide-rate diagnostics and a new tolerance parameter).
Add per-well feasibility checks based on guide-rate ratio and scaled phase fractions (from primary variables) to avoid singular control equations.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
opm/simulators/wells/WellInterface_impl.hpp	Adds debug logging for fallback usage; adds convenience overload to update fallback flag and calls it under GRUP control.
opm/simulators/wells/WellInterface.hpp	Adds a convenience overload and a new pure-virtual hook to retrieve scaled well fractions.
opm/simulators/wells/WellInterfaceGeneric.hpp	Declares base helper to update `use_group_target_fallback` using scaled fractions.
opm/simulators/wells/WellInterfaceGeneric.cpp	Implements fallback-flag decision logic based on guide-rate ratio and scaled well fractions.
opm/simulators/wells/WellGroupControls.cpp	Switches group-control equation and scaling to use either current target or fallback target based on the flag.
opm/simulators/wells/StandardWell_impl.hpp	Calls fallback-flag update before assembling control equations; wires scaled fractions from primary variables.
opm/simulators/wells/StandardWell.hpp	Implements required `getScaledWellFractions()` override for standard wells.
opm/simulators/wells/StandardWellPrimaryVariables.hpp	Declares helper to compute scaled phase fractions from current primary variables.
opm/simulators/wells/StandardWellPrimaryVariables.cpp	Implements scaled fraction extraction for standard wells.
opm/simulators/wells/MultisegmentWell_impl.hpp	Calls fallback-flag update before assembling control equations; wires scaled fractions from MSW primary variables.
opm/simulators/wells/MultisegmentWell.hpp	Implements required `getScaledWellFractions()` override for multisegment wells.
opm/simulators/wells/MultisegmentWellPrimaryVariables.hpp	Declares helper to compute scaled fractions for the top segment.
opm/simulators/wells/MultisegmentWellPrimaryVariables.cpp	Implements scaled fraction extraction for multisegment wells (top segment).
opm/simulators/wells/SingleWellState.hpp	Extends `GroupTarget` with mode + guide-rate diagnostics; adds fallback target + flag and serializes them.
opm/simulators/wells/SingleWellState.cpp	Initializes the new `use_group_target_fallback` member; extends equality to cover new fields.
opm/simulators/wells/GroupStateHelper.hpp	Adds helpers to map control mode to guide target, adds optional fallback cmode translation parameter, and adds prev-rate update API.
opm/simulators/wells/GroupStateHelper.cpp	Implements guide-target mapping by mode; computes guide-rate ratio diagnostics; translates targets using stored previous group production rates.
opm/simulators/wells/GroupState.hpp	Adds storage + serialization for previous production rates.
opm/simulators/wells/GroupState.cpp	Implements storage/accessors for previous production rates and extends equality.
opm/simulators/wells/FractionCalculator.hpp	Makes `guideRate()` available for diagnostics used by group target computation.
opm/simulators/wells/BlackoilWellModelGeneric.cpp	Updates group data flow to snapshot previous production rates and compute per-well fallback targets when group mode differs from scheduled/original mode.
opm/simulators/flow/BlackoilModelParameters.hpp	Adds `group_control_fraction_tolerance_` parameter to model parameters.
opm/simulators/flow/BlackoilModelParameters.cpp	Registers and loads the new `group_control_fraction_tolerance` parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-27T10:40:19Z

+                if (group_target.has_value()) {
+                    // check whether group giving target is operating at its original cmode, and if not, compute the target 
+                    // corresponding to original cmode for potential later use in case current cmode is/becomes infeasible
+                    const Group& target_group = this->schedule().getGroup(group_target->group_name, reportStepIdx);
+                    const auto cmode_orig = target_group.productionControls(summaryState_).cmode;
+                    if (cmode_orig != group_target->production_cmode &&
+                        cmode_orig != Group::ProductionCMode::FLD &&
+                        cmode_orig != Group::ProductionCMode::NONE) {
+                        auto& group_target_fallback = this->wellState().well(well->indexOfWell()).group_target_fallback;
+                        group_target_fallback = group_state_helper.getWellGroupTargetProducer(
+                            well->name(),
+                            well->wellEcl().groupName(),
+                            group,
+                            ws.surface_rates.data(),
+                            efficiencyFactor,
+                            resv_coeff,
+                            cmode_orig);
+                    }
+                }


group_target_fallback is only assigned when cmode_orig != group_target->production_cmode, but it is never cleared when that condition is false (or when group_target becomes nullopt). This can leave a stale fallback target from a previous iteration/step, and later logic (e.g., fallback-flag updates) may incorrectly use it or even assume group_target is present when only the stale fallback is set. Clear group_target_fallback (and ideally use_group_target_fallback) when no fallback is needed / when group_target is not set, to keep the well state consistent.

Made sure to reset group_target_fallback and flag

Copilot · 2026-03-27T10:40:19Z

+    auto& ws = well_state.well(this->index_of_well_);
+    if (!wellUnderGroupControl(ws) || this->isInjector() || !ws.group_target_fallback.has_value())
+    {
+        ws.use_group_target_fallback = false;
+        return;
+    }
+    // If group_target_fallback is not empty, group_target should not be empty either
+    assert(ws.group_target.has_value());
+
+    // We only need to check modes that can result in small fractions
+    // i.e. ORAT, WRAT, GRAT, LRAT
+    const auto cmode = ws.group_target->production_cmode;
+    const bool do_check = (cmode == Group::ProductionCMode::ORAT ||


This function relies on assert(ws.group_target.has_value()) before dereferencing ws.group_target. In release builds assertions are typically disabled, so if a stale group_target_fallback is present while group_target is empty, this becomes undefined behavior/crash. Add a runtime guard (e.g., treat as no-fallback / clear fallback) so dereferencing ws.group_target is always safe.

Removed assert, if no value function will now return early

Copilot · 2026-03-27T10:40:19Z

+GroupState<Scalar>::prev_production_rates(const std::string& gname) const
+{
+    auto group_iter = this->m_prev_production_rates.find(gname);
+    if (group_iter == this->m_prev_production_rates.end())
+        throw std::logic_error("No such group");
+
+    return group_iter->second;
+}
+
+template<class Scalar>
+void GroupState<Scalar>::update_prev_production_rates(const std::string& gname,
+                                                      const std::vector<Scalar>& rates)
+{
+    if (rates.size() != this->num_phases)
+        throw std::logic_error("Wrong number of phases");
+
+    this->m_prev_production_rates[gname] = rates;


The thrown std::logic_error messages here are very generic ("No such group" / "Wrong number of phases"). When these exceptions surface, it will be hard to diagnose which group triggered it and what sizes were expected/received. Include the group name (and expected vs actual phase counts) in the error message to make failures actionable.

Did not update this as multiple of functions here have exactly the same error logging

Copilot · 2026-03-27T10:40:19Z

+    Parameters ::Register<Parameters::GroupControlFractionTolerance<Scalar>>
+    ("Tolerance for minimal allowed fraction for controlled phase before employing a fallback control");


Parameter registration formatting is inconsistent with the rest of this function (Parameters ::Register has an extra space, and the continuation indentation differs). Keeping this consistent helps readability and avoids churn with auto-formatters.

Suggested change

Parameters ::Register<Parameters::GroupControlFractionTolerance<Scalar>>

("Tolerance for minimal allowed fraction for controlled phase before employing a fallback control");

Parameters::Register<Parameters::GroupControlFractionTolerance<Scalar>>

("Tolerance for minimal allowed fraction for controlled phase before employing a fallback control");

steink · 2026-03-27T12:19:12Z

Added a test where the PR has an efect here: OPM/opm-tests#1512

steink · 2026-03-27T13:31:47Z

jenkins build this failure_report please

GitPaean · 2026-04-09T09:53:23Z

jenkins build this failure_report please

GitPaean

This is a well structured pull request, and it fixes the issue as stated in OPM/opm-tests#1512 . I left some minor comments and questions.

GitPaean · 2026-04-09T11:24:22Z


+template<class FluidSystem, class Indices>
+void StandardWellPrimaryVariables<FluidSystem,Indices>::
+scaledWellFractions(std::vector<Scalar>& fractions) const


With this way, it might be a untouched 0 value fractions at the end of the std::vector<Scalar>& fractions. It should not be an issue, since we do not handle solvent with group control, I believe. And with group controls, there is no guide rate or target will be related to solvent either.

Thanks, might not have throught of this. Agree that it's not an issue with the current code, but might become an issue if the function should used for some other purpuse in the future. I can include the solvent-fraction if you think that's more clean or we can deal with it later if it becomes relevant?

I also struggle with solvent related sometimes. Maybe at least some comment for later record or even throw to say solvent related should not use this function if we want to leave this to be fixed later.

OK, I'll include a throw. Will update shortly, just need to resolve some building issues locally.

Now updated, please have a look and see if you agree.

GitPaean · 2026-04-09T11:45:59Z

                    this->well_control_log_.push_back(from);
                }
            }
+            // Add debug info for problematic group targets


With this way, we can only see this message when the local solve of the well equation is converged. It can be good to see the message whether the solving is converged or not.

Thanks, moved to end so message appears also for non-converged

GitPaean · 2026-04-09T11:50:31Z

 }

+template<typename Scalar, typename IndexTraits>
+void WellInterfaceGeneric<Scalar, IndexTraits>::updateGroupTargetFallbackFlag(WellState<Scalar, IndexTraits>& well_state,


It might be good to break the long line here, similar to other function definitions.

Thanks, done

GitPaean · 2026-04-09T11:51:06Z

+void WellInterfaceGeneric<Scalar, IndexTraits>::updateGroupTargetFallbackFlag(WellState<Scalar, IndexTraits>& well_state,
+                                                                              const std::vector<Scalar>& scaled_well_fractions,
+                                                                              DeferredLogger& deferred_logger) const
+    {


The indentation of the {.

GitPaean · 2026-04-09T12:50:09Z

+    // Finally, we provide the well-to-group guide-rate ratio for subsequent diagnostics of 
+    // target feasibility. Note that we cannot use the target directly for this, since a ~zero 
+    // target may very well just be the result of non-converged group tree. 
+    Scalar guide_rate_fraction = 0.0;


The guide_rate_fraction and also the guiderate_ratio in the class GroupTarget by default will 0. The usage is that if it is smaller than a small value (1.e-4 by default), it will use the fallback mode. Will initializing the values be 0. makes it easier to enter the fallback mode by accident? Will it be better to initialize it to be some big value?

Thanks, agree. Now intializes to 1.0

GitPaean · 2026-04-09T12:58:57Z

+    // Check whether guiderate ratio or well fraction of production_cmode is too small, 
+    // if so, switch to fallback (original) control mode
+    const Scalar fraction_tolerance = this->param_.group_control_fraction_tolerance_;
+    if (ws.group_target->guiderate_ratio < fraction_tolerance) {


I understand that why the well rate ratio for one phase is two low, it means it has the problem of singular well matrix. I do not understand well, why the small guiderate_ratio can also be a problem (I probably did not get it when you tried to explain to me). At least, it is not related to the singular well matrix, right? It does not look affecting the case in OPM/opm-tests#1512 .

Thanks, this is a good point. It might not be strictly neccessary, but the rationale was that we never attempt to solve a well with e.g., water-rate control if it has aprox zero water potential no matter what it's current (unconverged) water fraction is. I beleive this is more robust than only considering current fractions. I think that a well that cannot produce water, but for some reason is in a state with non-zero water fraction (in top segment), can result in a rather dramatic Newton update if solved with water constraint.

steink · 2026-04-10T09:02:37Z

jenkins build this failure_report please

steink · 2026-04-10T11:17:18Z

jenkins build this failure_report please

GitPaean

Thanks for the update. Leaving two small minor comments for you to address. Feel free to merge after jenkins approves.

GitPaean · 2026-04-10T11:42:36Z

+{
+    if constexpr (Indices::enableSolvent) {
+        OPM_DEFLOG_THROW(std::runtime_error,
+                         fmt::format("Function scaledWellFractions does not support solvent yet."),


I do not think fmt::format is necessary here.

Thanks, skipped

GitPaean · 2026-04-10T11:45:02Z

+        Scalar target_value{0.0};
+        Group::ProductionCMode production_cmode {Group::ProductionCMode::NONE};
+        Group::InjectionCMode injection_cmode {Group::InjectionCMode::NONE};
+        Scalar guiderate_ratio{0.0}; // well to group guide rate ratio for diagnostics


maybe there also should be 1.0, because by default it should not trigger the fallback mode.

Thanks, agreed. Fixed.

steink · 2026-04-10T12:02:37Z

jenkins build this failure_report please

bska · 2026-04-10T12:48:39Z

Did something go wrong in conflict resolution here? This PR appears to remove the new regression tests that were introduced in #6968. Was that intentional?

steink · 2026-04-10T13:06:52Z

Did something go wrong in conflict resolution here? This PR appears to remove the new regression tests that were introduced in #6968. Was that intentional?

Thanks for discovering! Absolutely not intentional.

bska · 2026-04-10T13:11:00Z

Did something go wrong in conflict resolution here? This PR appears to remove the new regression tests that were introduced in #6968. Was that intentional?

Thanks for discovering! Absolutely not intentional.

Thanks a lot for the quick response. I have restored those regression tests in PR #6980.

steink added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Mar 24, 2026

steink force-pushed the ensure_group_feasibility_tmp branch from 413e7e4 to a29892b Compare March 27, 2026 09:19

steink requested a review from GitPaean March 27, 2026 09:55

GitPaean requested a review from Copilot March 27, 2026 10:32

Copilot started reviewing on behalf of GitPaean March 27, 2026 10:33 View session

Copilot AI reviewed Mar 27, 2026

View reviewed changes

steink mentioned this pull request Mar 27, 2026

Test for handling non-feasible group-target OPM/opm-tests#1512

Merged

steink force-pushed the ensure_group_feasibility_tmp branch 2 times, most recently from 5e14009 to c5bf0f3 Compare March 27, 2026 13:30

GitPaean reviewed Apr 9, 2026

View reviewed changes

steink force-pushed the ensure_group_feasibility_tmp branch from bb17a4d to df601d2 Compare April 10, 2026 09:01

steink marked this pull request as ready for review April 10, 2026 09:02

steink force-pushed the ensure_group_feasibility_tmp branch from fa1cd05 to 54d7e0e Compare April 10, 2026 11:13

GitPaean approved these changes Apr 10, 2026

View reviewed changes

reworked implementation of group control feasibility

16227fc

steink force-pushed the ensure_group_feasibility_tmp branch from 99b0af7 to 16227fc Compare April 10, 2026 12:00

steink merged commit 8788b9f into OPM:master Apr 10, 2026
2 checks passed

bska mentioned this pull request Apr 10, 2026

Restore Accidentally Removed Regression Tests #6980

Merged

steink mentioned this pull request Apr 14, 2026

Switch to debug-logging #6986

Merged

		Parameters ::Register<Parameters::GroupControlFractionTolerance<Scalar>>
		("Tolerance for minimal allowed fraction for controlled phase before employing a fallback control");

Conversation

steink commented Mar 24, 2026

Uh oh!

steink commented Mar 24, 2026

Uh oh!

steink commented Mar 27, 2026

Uh oh!

steink commented Mar 27, 2026

Uh oh!

GitPaean commented Mar 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steink commented Mar 27, 2026

Uh oh!

steink commented Mar 27, 2026

Uh oh!

GitPaean commented Apr 9, 2026

Uh oh!

GitPaean left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steink commented Apr 10, 2026

Uh oh!

steink commented Apr 10, 2026

Uh oh!