Comp well improvements#7208
Merged
Merged
Conversation
GitPaean
added
the
manual:irrelevant
Member
Author
|
jenkins build this please |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the compositional well (“CompWell”) robustness and testability by extracting the wellbore flash/mass computations into reusable helpers, tightening handling of singular well Jacobian blocks, and adding targeted unit tests for both the flash derivative chain and the Schur-complement well equation machinery.
Changes:
- Extracts the wellbore PT-flash and component-mass computation into
CompWellFlash.hppand uses it fromCompWell. - Makes
CompWellEquations::invert()robust across multiple singular-matrix failure modes (exceptions and silent inf/NaN). - Adds two new unit tests covering flash AD-derivatives vs finite differences and Schur-complement operations / singular fallbacks.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_compwell_jacobian.cpp | New finite-difference vs AD-derivative regression test for wellbore flash/mass-fraction chain. |
| tests/test_compwell_equations.cpp | New unit test for Schur-complement operations and singular inverse fallback behavior. |
| flowexperimental/comp/wells/CompWellPrimaryVariables_impl.hpp | Generalizes clamping/renormalization of mole-fraction primaries. |
| flowexperimental/comp/wells/CompWellModel.hpp | Formatting/indentation cleanup. |
| flowexperimental/comp/wells/CompWellModel_impl.hpp | Clarifies report-step initialization behavior via comments. |
| flowexperimental/comp/wells/CompWellFlash.hpp | New extracted helpers: wellbore flash + per-component wellbore masses. |
| flowexperimental/comp/wells/CompWellEquations_impl.hpp | Detects singular inversion via exceptions and non-finite inverse entries; falls back to identity. |
| flowexperimental/comp/wells/CompWell.hpp | Includes the new CompWellFlash.hpp. |
| flowexperimental/comp/wells/CompWell_impl.hpp | Uses extracted helpers; adds explicit unsupported-case throw in getMobility; adjusts logging level. |
| CMakeLists_files.cmake | Registers the two new test sources in the build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
GitPaean
force-pushed
the
comp_well_improvements
branch
2 times, most recently
from
c0a23e1 to
9dd0990
Compare
Member
Author
|
jenkins build this please |
GitPaean
force-pushed
the
comp_well_improvements
branch
from
9dd0990 to
ac00220
Compare
Member
Author
|
jenkins build this please |
GitPaean
force-pushed
the
comp_well_improvements
branch
3 times, most recently
from
298bb27 to
0b3de1b
Compare
Member
Author
|
I think the well code change are good and clear improvements. The tests can be discussed. I am marking the PR as ready for review now. |
Member
Author
|
jenkins build this please |
GitPaean
force-pushed
the
comp_well_improvements
branch
from
0b3de1b to
ca067d7
Compare
Member
Author
|
jenkins build this please |
bska
reviewed
Jul 1, 2026
bska
left a comment
bska
left a comment
Member
There was a problem hiding this comment.
I think this looks good for the most part and I appreciate the simplification. Just a couple of surface level remarks from me.
is different from the connection cell's. throw for now, either zero mobility will be resulted.
Move the wellbore PT flash and the per-component mass computation out of CompWell into standalone flashWellboreFluidState()/wellboreComponentMasses() helpers (CompWellFlash.hpp), removing the duplicated mass loops in calculateExplicitQuantities()/updateTotalMass(). This makes the composition/pressure derivatives that flow through the flash unit-testable without instantiating a Simulator. Add tests/test_compwell_jacobian.cpp, which checks the AD derivatives of the component masses, mass fractions and fluid density against central finite differences. They agree to ~1e-7, confirming the previously-doubted mass-fraction derivatives are in fact correct.
…sizes CompWellEquations::invert() replaced a singular well matrix with the identity only when detail::invertMatrix() threw Opm::NumericalProblem, which happens just for the 4x4 block (three-component wells). For other sizes a singular matrix either throws Dune::FMatrixError (generic path) or silently produces inf/NaN (e.g. the 3x3 path used by two-component wells), so the fallback was bypassed and the Newton update became NaN. Catch both exception types and additionally detect a non-finite inverse. Add tests/test_compwell_equations.cpp covering the Schur-complement solve/recoverSolutionWell/apply against dense references, plus the singular fallback for both the 4x4 (throws) and 3x3 (silent inf/NaN) paths.
GitPaean
force-pushed
the
comp_well_improvements
branch
from
ca067d7 to
6f6a331
Compare
Member
Author
|
Thanks for the reviewing comments. I believe I have addressed them. |
Member
Author
|
jenkins build this please |
bska
approved these changes
Jul 1, 2026
bska
left a comment
bska
left a comment
Member
There was a problem hiding this comment.
Thanks a lot for the updates. This looks good to me now and I'll merge into master.
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.