Write compositional saturation pressure to restart output - #7389
Draft
GitPaean wants to merge 1 commit into
Draft
Conversation
GitPaean
added
the
manual:irrelevant
Member
Author
|
jenkins build this opm-common=5283 failure_report please |
GitPaean
force-pushed
the
comp_output_psat
branch
3 times, most recently
from
September 8, 2026 20:16
cc49680 to
037f51d
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The critical solver correctness issue and moderate testing and parallel-reporting issues remain unresolved.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds compositional PSAT restart output using phase pressure or the shared saturation-pressure solver.
Changes:
- Computes and exports saturation pressure.
- Adds conditional storage and failure reporting.
- Adds container allocation/export tests.
Review findings:
- Critical (1 vote): Solver failures can incorrectly report zero for near-critical cells.
- Moderate (1 vote): Tests do not cover phase selection or calculated values.
- Moderate (2 votes): Failure counts are rank-local rather than globally reduced.
File summaries
| File | Description |
|---|---|
tests/test_compositionalcontainer.cpp |
Tests PSAT allocation and export. |
opm/simulators/flow/OutputCompositionalModule.hpp |
Computes PSAT and reports solver failures; contains the unresolved findings. |
opm/simulators/flow/CompositionalContainer.hpp |
Declares PSAT storage APIs. |
opm/simulators/flow/CompositionalContainer.cpp |
Allocates and exports PSAT. |
CMakeLists_files.cmake |
Registers the new test. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 3
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
GitPaean
marked this pull request as draft
September 9, 2026 06:35
GitPaean
force-pushed
the
comp_output_psat
branch
from
September 9, 2026 07:57
cc9084b to
cb4e31d
Compare
GitPaean
force-pushed
the
comp_output_psat
branch
3 times, most recently
from
September 9, 2026 21:52
a1db652 to
d222608
Compare
GitPaean
changed the title
Handle PSAT requests in compositional restart output. Use the cell pressure when both hydrocarbon phases are present, solve the bubble or dew pressure for exact single-phase flash labels, and write zero when no boundary can be resolved. Allocate PSAT only for restart output and calculate it in the OpenMP cell loop. Make output validation virtual and run the MPI reductions after rank-wide exception handling, preventing a local failure from leaving ranks in different collectives. Reduce the failure count across ranks and log it once with clear singular and plural wording. Keep the phase decision independent of simulator state and test restart allocation, phase labels near round-off boundaries, solver results, and the failure contract.
GitPaean
force-pushed
the
comp_output_psat
branch
from
September 10, 2026 11:00
d222608 to
ef64102
Compare
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.
PSATinRPTRSTwas reported as unhandled in compositional runs. This change writes saturation pressure to restart output and allocates its buffer only when restart output requests it.For a cell whose flash reports both hydrocarbon phases,
PSATis the cell pressure. Exact liquid-only and vapour-only flash labels select a bubble- or dew-point solve for the total composition. If the solver cannot resolve a boundary, the output is zero and the log explains how many cells were affected.The per-cell calculation runs in the existing OpenMP fluid-in-place loop. Failures are counted atomically, reduced across MPI ranks, and logged once on rank zero. Output validation is virtual, and the collective reductions run only after rank-wide exception handling so a local exception cannot leave ranks entering different collectives.
Focused tests cover restart-only allocation, repeated allocation, exact and rounded phase labels, bubble/dew selection, two-phase pressure, and the failure contract (4 cases, 33 assertions). The exact OPM/opm-tests#1593 deck completes with one and eight OpenMP threads; its restart files are byte-identical and exercise 99 two-phase values, two solved single-phase values, and 39 explicit zero fallbacks. Combined with #7306 and OPM/opm-common#5349, both compositional equilibration decks also complete with one and four threads and produce byte-identical restart files with valid
PSATvalues.Requires the saturation-pressure solver in OPM/opm-common#5283. Regression coverage is proposed in OPM/opm-tests#1593. The paired Jenkins build passed 969 tests with no compiler warnings before the final history cleanup.