Let the classic CPRW contract D for multisegment wells - #7282
Conversation
hnil
added
the
manual:enhancement
| 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) { |
There was a problem hiding this comment.
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)?
There was a problem hiding this comment.
Added an assert(lambda.size() <= Block::rows) so a future reordering of the well primary variables trips instead of silently contracting the wrong rows.
| /// the coarse pressure system singular. | ||
| template <class DiagMatWell, class WellWeight> | ||
| typename DiagMatWell::field_type | ||
| contractCprWellDiagonal(const DiagMatWell& D, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
| // 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"); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
ElyesAhmed
left a comment
ElyesAhmed
left a comment
There was a problem hiding this comment.
This is Independent of the other two PRs so its better. This one is completely fine
|
All three taken, thanks.
Unrelated, found while testing this: |
|
lies. |
330e23f to
8b12de9
Compare
|
jenkins build this please |
|
jenkins build this failure_report please |
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>
8b12de9 to
8b01b92
Compare
|
jenkins build this failure_report please |
4 participants
Standard wells build their coarse CPRW diagonal by contracting D
(
StandardWellEquationsreadsduneD_[0][0]). Multisegment wells instead useminus 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_dmakes the multisegment pathcontract D as well. Default
autois today's behaviour, so nothing changes unlessasked 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 ismswellhelpers::contractCprWellDiagonal, unit tested intest_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:cprwcprw,well_coarse_diagonal = contract_dStandard wells are byte-identical either way, as they must be.
🤖 Generated with Claude Code
Note on the
well_coarse_diagonalconfig keyThis PR's classic path (
PressureBhpTransferPolicy) accepts{auto, contract_d}and defaults toauto; #7278's system path accepts{auto, contract_d, row_sum}and defaults tocontract_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.