Skip to content

Expose hardcoded well/Newton tuning constants; remove the dead tolerance-well-control knob - #7300

Open
hnil wants to merge 4 commits into
OPM:masterfrom
hnil:expose-well-newton-constants
Open

Expose hardcoded well/Newton tuning constants; remove the dead tolerance-well-control knob#7300
hnil wants to merge 4 commits into
OPM:masterfrom
hnil:expose-well-newton-constants

Conversation

@hnil

@hnil hnil commented Aug 9, 2026

Copy link
Copy Markdown
Member

Makes parameters of values that could previously only be studied by rebuilding: the standard-well control-equation tolerances (defaults = the hardcoded values; note StandardWell THP was 10x looser than MSW with no stated reason), the forced iterations between control switches in the inner well solve, the Newton damping schedule, and the stopped/dynamic-THP well tolerance factors.

Also removes --tolerance-well-control, which was registered and read but never used anywhere, and documents the -1 sentinel on --min-strict-cnv-iter the way the MB twin already does. Byte-identical at defaults on the decks tested.

🤖 Generated with Claude Code

@hnil hnil added the manual:enhancement This is an enhancement/improvent that needs to be documented in the manual label Aug 10, 2026
@hnil
hnil requested review from GitPaean and bska August 10, 2026 09:02
@hnil
hnil marked this pull request as ready for review August 10, 2026 09:03
@GitPaean
GitPaean requested a balanced review from Copilot August 10, 2026 09:14
@GitPaean

GitPaean commented Aug 10, 2026

Copy link
Copy Markdown
Member

jenkins build this failure_report please

Copilot AI reviewed Aug 10, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Exposes well-convergence and Newton-tuning constants as runtime parameters while removing an unused well-control tolerance option.

Changes:

  • Adds configurable standard/MSW well tolerances and switch intervals.
  • Exposes Newton damping schedule parameters.
  • Clarifies CNV behavior and the -1 sentinel documentation.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
opm/simulators/wells/StandardWellEval.hpp Extends convergence API with control tolerances.
opm/simulators/wells/StandardWellEval.cpp Applies configurable control tolerances.
opm/simulators/wells/StandardWell_impl.hpp Uses configurable factors and switch intervals.
opm/simulators/wells/MultisegmentWell_impl.hpp Configures MSW switch intervals.
opm/simulators/flow/NonlinearSystemBlackOilReservoir_impl.hpp Clarifies CNV override behavior.
opm/simulators/flow/NonlinearSolver.hpp Defines Newton tuning parameters.
opm/simulators/flow/NonlinearSolver.cpp Registers and loads Newton parameters.
opm/simulators/flow/BlackoilModelParameters.hpp Defines new well parameters and removes the dead option.
opm/simulators/flow/BlackoilModelParameters.cpp Registers and loads well parameters.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread opm/simulators/flow/BlackoilModelParameters.hpp
@hnil
hnil force-pushed the expose-well-newton-constants branch from 5019e8a to a26f539 Compare August 11, 2026 08:26
@hnil

hnil commented Aug 11, 2026

Copy link
Copy Markdown
Member Author

The build break was real: flowexperimental/flowexp.hpp still had a Parameters::Hide<> for the removed knob. Dropped there too, and rebased on master.

@GitPaean

GitPaean commented Aug 11, 2026

Copy link
Copy Markdown
Member

jenkins build this failure_report please

hnil and others added 4 commits August 11, 2026 14:05
@hnil
Make the standard-well control equation tolerances parameters
0d829a3 
The tolerances for the standard-well control equation residual were
hardcoded as a brace-initialised literal at the call site,
{1e3, 1e4, 1e-4, 1e-6}, i.e. BHP 0.01 bar, THP 0.1 bar, rates 1e-4 m3/s
and group 1e-6 m3/s. Multisegment wells have had the equivalent values as
parameters all along (and use a 10x stricter pressure tolerance), so the
two well types could not be configured or studied consistently.

Add --tolerance-well-{bhp,thp,grup}-eq with defaults equal to the previous
literals, and reuse the existing --tolerance-wells for the rate entry
(its default already matched the literal). Purely a plumbing change: runs
are numerically identical at default settings.
@hnil @claude
Expose hardcoded convergence/well tuning constants as parameters
3aab09d 
Five values that steer the nonlinear and well solves were written at the call
site and could not be studied without a rebuild:

  --min-iter-after-switch-wells (4) / --min-iter-after-switch-ms-wells (3)
      Iterations forced between two control switches in the inner well solve.
      The existing comment notes that values <= 1 make the solve cycle, i.e.
      a tuning constant with a known failure mode.  Standard and multi-segment
      wells used different values with no stated reason.
  --newton-relax-increment (0.1) / --newton-relax-rel-tol (0.2)
      The Newton oscillation-damping schedule.  Set in reset() only, so
      --newton-max-relax could be tuned but the schedule reaching it could not.
  --tolerance-wells-stopped-factor (1e-4) / --tolerance-wells-dynamic-thp-factor (1e-1)
      Multipliers on the well tolerance for stopped/zero-rate wells and for
      wells on a dynamic THP limit.

Defaults reproduce the previous values; INFOSTEP is unchanged on
SPE1CASE2_RADIAL, STONE2_MOD1, 3B_WSEGVALV_MODEL3, 2_GCONINJE_NETV and
GASLIFT-06.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hnil @claude
Remove the dead --tolerance-well-control knob and document the -1 sen…
2d602f8 
…tinel

--tolerance-well-control ("Tolerance for the well control equations") is
registered and read into tolerance_well_control_, but that member is never
used anywhere.  It is precisely the knob a user reaches for to tighten the
well control equations, and it silently does nothing; the real values live in
--tolerance-well-{bhp,thp,grup}-eq.

Also:
  - --min-strict-cnv-iter now documents that the default -1 is a mode switch
    (relax only at the iteration cap), matching the text --min-strict-mb-iter
    already carries for the same sentinel.  Read plainly, "minimum number of
    iterations before relaxed tolerances can be used ... -1" suggests the
    opposite of what it does.
  - Correct the comment on the solution-change branch of the CNV test: 1e20
    disables CNV rather than relaxing it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@hnil @claude
flowexp: stop hiding the removed tolerance-well-control parameter
7209421 
The knob is dead as a tolerance, but flowexp still registered a Hide for
it, so dropping the parameter broke that build.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hnil
hnil force-pushed the expose-well-newton-constants branch from a26f539 to 7209421 Compare August 11, 2026 12:05
@GitPaean

GitPaean commented Aug 11, 2026

Copy link
Copy Markdown
Member

jenkins build this failure_report please

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@bska bska Awaiting requested review from bska
@GitPaean GitPaean Awaiting requested review from GitPaean

Assignees

No one assigned

Labels

manual:enhancement This is an enhancement/improvent that needs to be documented in the manual

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hnil @GitPaean