Let the classic CPRW contract D for multisegment wells by hnil · Pull Request #7282 · OPM/opm-simulators

hnil · 2026-08-05T12:45:37Z

Standard wells build their coarse CPRW diagonal by contracting D
(StandardWellEquations reads duneD_[0][0]). Multisegment wells instead use
minus the row sum of the contracted reservoir entries, which never reads D and so
drops every segment-to-segment coupling — most of the well once there is one
segment per connection.

preconditioner.well_coarse_diagonal = contract_d makes the multisegment path
contract D as well. Default auto is today's behaviour, so nothing changes unless
asked for. Contracting D is also the Galerkin diagonal for the prolongation the
coarse column already assumes: one coarse value spread over the well's segment
pressures.

The flag is threaded next to use_well_weights. The contraction is
mswellhelpers::contractCprWellDiagonal, unit tested in test_MswCprWellDiagonal;
it returns 1 on exact cancellation, since a zero would make the coarse pressure
system singular.

Full Norne with --convert-to-multisegment-well=per-connection, linear iterations:

	linear iterations
`cprw`	2716
`cprw`, `well_coarse_diagonal = contract_d`	2646

Standard wells are byte-identical either way, as they must be.

🤖 Generated with Claude Code

Note on the `well_coarse_diagonal` config key

This PR's classic path (PressureBhpTransferPolicy) accepts {auto, contract_d} and defaults to auto; #7278's system path accepts {auto, contract_d, row_sum} and defaults to contract_d. Same key name, different valid set and different default — each preserving its own historical behaviour, which is why they differ. Recorded here so that if the two paths are ever unified, the divergence is a known decision rather than a surprise.

ElyesAhmed · 2026-08-10T08:15:25Z

+        using Scalar = typename DiagMatWell::field_type;
+        Scalar diag = 0.0;
+        for (std::size_t row = 0; row < D.N(); ++row) {
+            for (auto col = D[row].begin(), end = D[row].end(); col != end; ++col) {


maybe not correct but the contraction reads only the first lambda.size() (= numEq) rows of each D block, which is correct and I think consistent with the B-contraction above, but it silently assumes the first numEq rows of D are the conservation equations aligned with lambda. Could we add a cheap guard to catch a future reordering of well primary variables, e.g. assert(lambda.size() <= Block::rows) (or a one-line comment reaffirming the row ordering)?

Added an assert(lambda.size() <= Block::rows) so a future reordering of the well primary variables trips instead of silently contracting the wrong rows.

ok but maybe not complete if we reorders the well primary variables ..! One can move the segment pressure equation to the first row instead of the last, or inserting an extra equation before the conservation rows – this function would silently contract the wrong rows without any indication...maybe add "if (lambda.size() > static_caststd::size_t(Block::rows)) {
OPM_THROW(std::logic_error,
"contractCprWellDiagonal: lambda size (" + std::to_string(lambda.size()) +
") exceeds block rows (" + std::to_string(Block::rows) +
"). The first rows of D must be the conservation equations.");
}"
Maybe this is not necessary but good to leave at least a comment

ElyesAhmed · 2026-08-10T08:17:49Z

+    /// the coarse pressure system singular.
+    template <class DiagMatWell, class WellWeight>
+    typename DiagMatWell::field_type
+    contractCprWellDiagonal(const DiagMatWell& D,


this implements the same contraction as the inline D-loop in SystemCprwPressureStage::assembleCoarseMatrix (#7278), just over Dune BCRSMatrix instead of the merged blocks. I dont know if we need to unify them or no, but we can add // keep in sync with SystemCprwPressureStage-style comment on both sides would help whoever touches one later.

Agreed they are the same contraction. Added a cross-reference comment here rather than unifying - the two operate on different types (Dune BCRSMatrix vs merged blocks) and #7278 is not merged, so a shared helper would have to land there first.

Looked at unifying them properly and I do not think one templated function serves both. They share the inner kernel (sum_i lambda[i] * block[i][col]), but this one accumulates a single well's D into one scalar with one weight vector, while the system stage distributes into a whole coarse row using per-block weights and a block-to-well map. Making one function do both means passing the weights per block plus a destination callback - more machinery than the three-line loop it would replace. I have corrected the comment here, which oversold the similarity and is what prompted the question.

ElyesAhmed · 2026-08-10T08:20:54Z

+            // for every well; "auto" (the default) leaves the multisegment path
+            // on its historical row-sum diagonal. Standard wells contract D
+            // either way, so this only changes multisegment wells.
+            const auto diagonal = prm_.get<std::string>("well_coarse_diagonal", "auto");


Maybe this is fixed somewhere else or in other PRs: but this path accepts {auto, contract_d} and defaults to auto, while #7278's system path (wellCoarseDiagonalFromString) accepts {auto, contract_d, row_sum} and defaults to contract_d. Same config key name, different valid set and default. Each preserving its own historical default is fine, but worth one line in the PR description so if we are tuning both paths we understand that how row_sum is rejected

Real divergence, and deliberate here: row_sum only makes sense for the merged system matrix, so the classic path has no meaningful implementation of it. Noted in the PR description so it is not read as an oversight.

Taken further: auto is gone. The classic path now accepts {row_sum, contract_d} with row_sum as the behaviour-preserving default, so it says which diagonal it forms instead of hiding it behind a well-type test, and it uses the same words as the system path. row_sum still is not offered for standard wells there - they contract D unconditionally and the flag is ignored for them.

ElyesAhmed

This is Independent of the other two PRs so its better. This one is completely fine

hnil · 2026-08-10T09:42:39Z

All three taken, thanks.

Guard added: assert(lambda.size() <= Block::rows), with a comment saying it is there to catch a reordering of the well primary variables that moves something other than the conservation equations into the first rows.
Cross-reference added at contractCprWellDiagonal pointing at the equivalent inline loop in SystemCprwPressureStage::assembleCoarseMatrix, so whoever changes one finds the other. Not unifying them yet — the system path is still moving in Add a CPRW pressure stage to the system solver #7278.
The config-key divergence is now in the PR description: classic accepts {auto, contract_d} defaulting to auto, system accepts {auto, contract_d, row_sum} defaulting to contract_d. Each keeps its own historical default; written down so a future unification treats it as a known decision.

Unrelated, found while testing this: SPE1CASE2_GASWATER_MSW trips a pre-existing assert at StandardWell_impl.hpp:67 (num_conservation_quantities_ == numWellConservationEq) in an assert-enabled build. It fires without this branch too, and CI does not see it because CI builds with NDEBUG.

akva2 · 2026-08-10T09:44:41Z

lies.

bska · 2026-08-11T09:53:16Z

jenkins build this please

bska · 2026-08-11T11:05:55Z

jenkins build this failure_report please

bska · 2026-08-11T15:50:39Z

jenkins build this failure_report please

bska · 2026-08-27T11:30:52Z

jenkins build this failure_report please

Copilot

Pull request overview

This PR introduces an opt-in configuration for the classic CPRW path to build the multisegment well coarse diagonal by contracting the D block (matching standard wells), preserving segment-to-segment coupling that is otherwise dropped by the historical row-sum convention.

Changes:

Thread a new contract_d_diagonal boolean through well-pressure-equation assembly so multisegment wells can optionally contract D for the coarse diagonal.
Add preconditioner.well_coarse_diagonal parsing/validation in PressureBhpTransferPolicy and set a default in the CPRW property-tree setup.
Add mswellhelpers::contractCprWellDiagonal() plus a dedicated unit test test_MswCprWellDiagonal.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/test_MswCprWellDiagonal.cpp	Adds unit tests for the new multisegment coarse-diagonal contraction helper.
opm/simulators/wells/WellInterface.hpp	Extends the well interface method signature to carry the new diagonal-selection flag.
opm/simulators/wells/StandardWellEquations.hpp	Threads the new flag through the standard-well equation interface (unused there).
opm/simulators/wells/StandardWellEquations.cpp	Accepts the new parameter (ignored) and updates template instantiation signature.
opm/simulators/wells/StandardWell.hpp	Updates `StandardWell` override signature to include the new flag.
opm/simulators/wells/StandardWell_impl.hpp	Passes the new flag through to `StandardWellEquations::extractCPRPressureMatrix`.
opm/simulators/wells/MultisegmentWellEquations.hpp	Adds the new flag to the multisegment pressure-matrix extraction signature.
opm/simulators/wells/MultisegmentWellEquations.cpp	Implements optional `D` contraction for multisegment coarse diagonal when enabled.
opm/simulators/wells/MultisegmentWell.hpp	Updates `MultisegmentWell` override signature to include the new flag.
opm/simulators/wells/MultisegmentWell_impl.hpp	Passes the new flag into multisegment CPRW pressure-matrix extraction.
opm/simulators/wells/MSWellHelpers.hpp	Adds `contractCprWellDiagonal()` helper used for multisegment coarse diagonal contraction.
opm/simulators/wells/BlackoilWellModelNldd.hpp	Extends NLDD well-model interface signature with the new flag.
opm/simulators/wells/BlackoilWellModelNldd_impl.hpp	Updates NLDD stub implementation signature (still throws unimplemented).
opm/simulators/wells/BlackoilWellModel.hpp	Extends `BlackoilWellModel` API to carry the new flag into domain routing.
opm/simulators/wells/BlackoilWellModel_impl.hpp	Passes the new flag through to per-well pressure-equation assembly.
opm/simulators/linalg/WellOperators.hpp	Extends linear-operator wrappers to forward the new parameter.
opm/simulators/linalg/setupPropertyTree.cpp	Sets default `preconditioner.well_coarse_diagonal` for CPRW configs.
opm/simulators/linalg/PressureBhpTransferPolicy.hpp	Parses/validates `well_coarse_diagonal` and forwards selection to well assembly.
CMakeLists_files.cmake	Registers the new unit test in the CMake test sources list.

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

+                }
+            }
+        }
+        return (std::abs(diag) > 0.0) ? diag : Scalar{1.0};


+        // lambda indexes the conservation-equation rows of a D block, so the
+        // weights must not outrun them.  Catches a future reordering of the
+        // well primary variables that puts something else in the first rows.
+        assert(lambda.size() <= static_cast<std::size_t>(Block::rows));
+


+            const auto diagonal = prm_.get<std::string>("well_coarse_diagonal", "row_sum");
+            if (diagonal != "row_sum" && diagonal != "contract_d") {
+                OPM_THROW(std::invalid_argument,
+                          "Unknown well_coarse_diagonal '" + diagonal +
+                          "'. Valid values are 'row_sum' and 'contract_d'.");
+            }
+            const bool contract_d_diagonal = (diagonal == "contract_d");
+            fineOperator.addWellPressureEquations(*coarseLevelMatrix_, weights_,
+                                                  use_well_weights, contract_d_diagonal);


preconditioner.well_coarse_diagonal = contract_d takes a well's coarse diagonal from lambda' D(:,p) instead of minus the row sum of its contracted reservoir entries. Default "auto" keeps today's behaviour. Standard wells have always contracted D (StandardWellEquations reads duneD_[0][0]); only the multisegment path used the row sum, which never reads D and therefore throws away all segment-to-segment coupling. With one segment per connection that is most of the well. Contracting D is also the Galerkin diagonal for the prolongation the coarse column already assumes -- one coarse value spread over all of the well's segment pressures. The flag is threaded next to use_well_weights, from PressureBhpTransferPolicy down to MultisegmentWellEquations::extractCPRPressureMatrix. The contraction itself is mswellhelpers::contractCprWellDiagonal, unit tested in test_MswCprWellDiagonal; it returns 1 on exact cancellation, since a zero would make the coarse pressure system singular. Norne with --convert-to-multisegment-well=per-connection: every coarse matrix differs from the default, i.e. the flag reaches the assembly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> (cherry picked from commit 2e00bc3)

- assert(lambda.size() <= Block::rows): the weights index the conservation-equation rows of a D block, so a future reordering of the well primary variables that moves something else into the first rows is caught rather than silently contracting the wrong entries. - Note pointing at the equivalent inline loop in SystemCprwPressureStage::assembleCoarseMatrix, so whoever changes one finds the other. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

The classic path accepted {auto, contract_d} where "auto" meant "row sum for multisegment wells". Say row_sum instead: a configuration then states which diagonal it wants, and the key uses the same vocabulary as the system solver's. Default unchanged in behaviour. Also corrects the cross-reference on contractCprWellDiagonal: the system solver runs the same inner kernel but distributes into a whole coarse row with per-block weights, so it is not the same function under a template. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

hnil · 2026-09-18T07:26:04Z

jenkins build this please

hnil added the manual:enhancement This is an enhancement/improvent that needs to be documented in the manual label Aug 5, 2026

ElyesAhmed reviewed Aug 10, 2026

View reviewed changes

hnil force-pushed the pr/msw-cprw-contract-d branch from 330e23f to 8b12de9 Compare August 11, 2026 08:23

hnil force-pushed the pr/msw-cprw-contract-d branch from 8b12de9 to 8b01b92 Compare August 11, 2026 12:06

hnil requested review from ElyesAhmed and removed request for ElyesAhmed August 20, 2026 09:51

bska requested a lite review from Copilot August 27, 2026 11:30

Copilot started reviewing on behalf of bska August 27, 2026 11:31 View session

Copilot AI reviewed Aug 27, 2026

View reviewed changes

hnil requested a review from ElyesAhmed August 28, 2026 13:58

hnil mentioned this pull request Sep 15, 2026

System CPRW: THP well treatment and trivial-row scaling #7316

Draft

hnil and others added 3 commits September 18, 2026 09:17

hnil force-pushed the pr/msw-cprw-contract-d branch from f44678a to 6e2957f Compare September 18, 2026 07:26

Conversation

hnil commented Aug 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Note on the well_coarse_diagonal config key

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!

ElyesAhmed Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

ElyesAhmed left a comment

Choose a reason for hiding this comment

Uh oh!

hnil commented Aug 10, 2026

Uh oh!

akva2 commented Aug 10, 2026

Uh oh!

bska commented Aug 11, 2026

Uh oh!

bska commented Aug 11, 2026

Uh oh!

bska commented Aug 11, 2026

Uh oh!

bska commented Aug 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

hnil commented Sep 18, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

hnil commented Aug 5, 2026 •

edited

Loading

Note on the `well_coarse_diagonal` config key

ElyesAhmed Aug 10, 2026 •

edited

Loading