adding saturation pressure solver - #5283
Draft
GitPaean wants to merge 8 commits into
Draft
Conversation
GitPaean
added
the
manual:irrelevant
Member
Author
|
jenkins build this please |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds compositional bubble/dew saturation-pressure solving and fixes defaulted EQUIL item 11 handling.
Changes:
- Adds a successive-substitution saturation-pressure solver.
- Adds reference-based solver tests.
- Handles omitted EQUIL item 11 as enabling saturation pressure.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
opm/material/constraintsolvers/SaturationPressure.hpp |
Implements bubble/dew pressure solving. |
tests/material/test_saturation_pressure.cpp |
Tests reference saturation results. |
opm/input/eclipse/EclipseState/InitConfig/Equil.cpp |
Handles defaulted EQUIL item 11. |
tests/parser/InitConfigTest.cpp |
Tests defaulted item handling. |
CMakeLists_files.cmake |
Registers the solver and tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
GitPaean
force-pushed
the
comp_equil_july_6th
branch
from
August 10, 2026 12:42
fb292b0 to
55677b0
Compare
GitPaean
force-pushed
the
comp_equil_july_6th
branch
2 times, most recently
from
August 18, 2026 21:21
19ba79f to
8f92c0f
Compare
Member
Author
|
jenkins build this please |
GitPaean
force-pushed
the
comp_equil_july_6th
branch
2 times, most recently
from
August 22, 2026 21:47
01aafb6 to
aa45c19
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Suppressed comments (1)
opm/material/constraintsolvers/SaturationPressure.hpp:243
- This marks the pressure as two-phase even when the fixed-pressure substitution exhausted
maxInnerwithout converging. The subsequentsumand pressure step then use an unconvergedK, which can poison the bracket and make the outer loop miss or misidentify the saturation boundary. Handle!substitutionConvergedbefore updating either bracket endpoint (by retrying with a fresh estimate or reporting failure).
pTwo = p;
haveTwo = true;
GitPaean
force-pushed
the
comp_equil_july_6th
branch
from
August 24, 2026 13:57
67bc65f to
fe99a9f
Compare
Member
Author
|
jenkins build this opm-simulators=7306 please |
Member
Author
|
jenkins build this opm-simulators=7306 failure_report please |
Member
Author
|
jenkins build this failure_report please |
GitPaean
force-pushed
the
comp_equil_july_6th
branch
from
August 28, 2026 19:24
1b9f827 to
01c6dc3
Compare
The upper and lower dew searches were chained with ||, so a plain convergence failure on the upper branch silently fell through to the lower one, which can return the wrong dew point of a mixture that has both. The branch search now reports whether it converged, established that the branch has no genuine root, or merely ran out of iterations, and the lower branch is only tried in the second case.
EQUIL item 11 set to 1 was never exercised; the trivial-solution test could pass without running any assertion when both mixtures are simply refused.
A pressure was recorded as two-phase bracket evidence before the converged point was classified, so the disguised-trivial rejection bisected between two copies of the same pressure and froze, and an exhausted substitution could certify a pressure it proved nothing about. Both labels now require a converged substitution and follow the classification. The root-distinctness test accepted the 1e-7 m3/mol clamp the cubic EOS substitutes for an unphysical root as if it were a real phase, inventing a 42 kbar bubble point for supercritical methane; a volume at the clamp no longer counts. Found by the new supercritical test, which now expects a refusal with the pressure output untouched. Also bound the lower-branch dew test away from the retrograde region, align the Outcome enumerators with the neighbouring enum, and document the failure contract of the public solvers.
A scan point whose fixed-pressure substitution runs out of iterations classifies nothing. Two places still treated such a point as evidence: the single-phase test, whose trivial threshold of 1e-5 an unconverged K can meet while it is still moving, and the final verdict, which reported no root when the scan had merely failed. Both now require a converged substitution, as the pressure criterion already did. A search that met an exhausted point returns GaveUp, so the upper dew search no longer hands a mixture that has both branches to the lower one.
The previous commit refused the verdict whenever any scan point ended with the substitution exhausted. That is too strict to be usable: on a methane-rich mixture with no retrograde dew point, 194 of the 200 scan points of the upper-dew search are exhausted, and the search still correctly establishes that the branch has no root. Refusing there took the lower-branch fallback away from a mixture that needs it, and dewPressure() returned nothing at all. The verdict now rests on haveSingle: at least one pressure classified as single phase by a converged substitution. A scan that classified nothing anywhere has not looked, and still gives up rather than reporting no root.
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.
This adds a saturation pressure solver for compositional mixtures, computing bubble-point and dew-point pressures by successive substitution on fugacity coefficient ratios; the dew-point search targets the retrograde branch first, to match the saturation pressure of a gas reported by the reference simulator.
It also fixes the handling of a defaulted item 11 of EQUIL in compositional mode, which previously threw because the item has no default value.
Both are needed by the compositional equilibration with EQUIL and ZMFVD in an upcoming opm-simulators PR; the unit tests are anchored to reference simulator results for a 1D vertical equilibration case.