Skip to content

Give the SSHIFT volume shift to the saturations - #7391

Draft
GitPaean wants to merge 8 commits into
OPM:masterfrom
GitPaean:comp_sshift_saturations
Draft

Give the SSHIFT volume shift to the saturations#7391
GitPaean wants to merge 8 commits into
OPM:masterfrom
GitPaean:comp_sshift_saturations

Conversation

@GitPaean

@GitPaean GitPaean commented Sep 8, 2026

Copy link
Copy Markdown
Member

Depends on OPM/opm-common#5322 : this calls correctedMolarVolume(), which that PR adds, so it does not build on its own.

Saturations are volume fractions, but they were built from the unshifted molar volume while the densities beside them carried the shift. An SSHIFT deck got corrected densities next to phase volumes that did not match them. The reservoir path and the wellbore flash both did this; both now take the corrected volume. The compressibility factors stay on the unshifted equation-of-state root, which is what the single-phase label check reads.

Verified against reference runs of the same deck with and without the shift: the saturation response to the shift was zero on our side and 0.0135 on the reference's, and now matches to the digits printed. Densities and viscosities are unchanged.

Adds a regression test on compositional/SIMPLE_COMP_SSHIFT.DATA and a nonzero-shift case to the wellbore derivative test. The regression test fails until its reference data is generated.

@GitPaean GitPaean added the manual:irrelevant This PR is a minor fix and should not appear in the manual label Sep 8, 2026
@GitPaean
GitPaean force-pushed the comp_sshift_saturations branch 2 times, most recently from 6bae2de to 2b23a32 Compare September 8, 2026 13:03
Build volume fractions from SSHIFT-corrected phase volumes so reservoir
saturations are consistent with the shifted densities. This uses
correctedMolarVolume() from opm-common PR OPM#5322.
Register compositional/SIMPLE_COMP_SSHIFT.DATA for output comparison
with flow_comp, using flow_comp3_2p as the development simulator.
The three-component CO2 injection case uses shifts of both signs and
exercises their effect on density, viscosity and phase volumes.
Separate the compressibility factors stored in the fluid state from the
translated volume factors used to calculate saturations. Compute the
stored factors from the unshifted cached molar volume to retain the EOS
definition Z = p V_EOS / (R T).
Use L V_L / (L V_L + (1 - L) V_V) for the hydrocarbon liquid volume
fraction. The p/(R T) factors in Z cancel only when they are equal
between phases; molar volumes express the volume fraction directly
without that assumption.
Reservoir saturations now use corrected molar volumes directly. Remove
the intermediate translated Z values and keep the EOS factors stored in
the fluid state. Print Vm_L and Vm_V in the verbose saturation output.
Use corrected molar volumes for wellbore saturations so component
storage and mixture density use consistent phase volumes and densities.
Preserve the unshifted EOS factors used by the single-phase label check.
Allow test component registration with configurable SSHIFT values. Run
the AD-versus-central-difference checks with shifts of both signs,
alongside the existing zero-shift case.

Require a change in mixture density to verify that the shifted
configuration is active before checking its derivatives.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The regression lacks reference outputs, its tolerance masks the target signal, and the wellbore test does not directly verify saturation changes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Applies SSHIFT-corrected molar volumes when calculating compositional phase saturations in reservoir and wellbore flashes.

Changes:

  • Uses translated molar volumes for oil/gas saturation calculations.
  • Preserves unshifted EOS roots for compressibility factors.
  • Adds wellbore derivative and compositional regression coverage.
File summaries
File Description
opm/models/ptflash/flashintensivequantities.hh Corrects reservoir saturations.
flowexperimental/comp/wells/CompWellFlash.hpp Corrects wellbore saturations.
tests/test_compwell_jacobian.cpp Adds shifted wellbore testing.
regressionTests.cmake Registers the SSHIFT regression case.
Review details
  • Files reviewed: 4/4 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.

Comment thread regressionTests.cmake
Comment thread regressionTests.cmake Outdated
Comment thread tests/test_compwell_jacobian.cpp Outdated
@GitPaean
GitPaean force-pushed the comp_sshift_saturations branch from 0d71746 to c4fee29 Compare September 8, 2026 20:26
Record oil and gas saturations in the wellbore test helper and require
them to change when SSHIFT is enabled. Check that both phases are
present before testing derivatives.

Use an absolute tolerance of 1e-3 for SIMPLE_COMP_SSHIFT so the default
0.02 tolerance cannot mask small saturation changes.

Validation: both compositional well tests pass. Reverting the wellbore
saturation translation makes the new assertion fail. SSHIFT output
matches a saved local run at the tighter tolerance; official reference
generation remains a CI prerequisite.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Final validation depends on an unmerged opm-common API change and pending regression reference data.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

manual:irrelevant This PR is a minor fix and should not appear in the manual

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants