Add per-region SALINITY support for CO2STORE multi-PVT decks by dlmorenob · Pull Request #5107 · OPM/opm-common

dlmorenob · 2026-04-12T23:55:17Z

Summary

The SALINITY keyword currently accepts only a single record, so all PVT regions in a CO2STORE model use the same brine salinity. This prevents accurate modelling of stacked
aquifer storage targets with different formation-water TDS, or formations with caprock/reservoir salinity contrasts.

This PR allows one SALINITY record per PVT region (size tied to TABDIMS item NTPVT, mirroring how PVTW is handled).

Changes

share/keywords/001_Eclipse300/S/SALINITY — keyword size: 1 → {"keyword": "TABDIMS", "item": "NTPVT"}
Co2StoreConfig.hpp — double salt → std::vector<double> salt; added salinity(regionIdx) and numSalinityRegions() accessors
Co2StoreConfig.cpp — parser loops over records; backward-compat salinity() returns salt[0]; out-of-range regionIdx broadcasts salt[0]
BrineCo2Pvt.cpp — salinity() → salinity(regionIdx) in per-region init loop
Co2GasPvt.cpp — same one-line fix for gas-phase PVT path
tests/parser/EclipseStateTests.cpp — added TABDIMS / to existing SALTMFTest

Backward compatibility

A deck with a single SALINITY record (the only form currently in use) works unchanged. NTPVT defaults to 1, salinity() (no args) returns salt[0], and salinity(regionIdx)
broadcasts salt[0] for out-of-range indices.

Testing

23 unit-test assertions (single/multi-record, SALTMF, edge cases)
Full ctest: 251/252 pass (1 pre-existing python_tests failure on clean master)
CO2STORE simulations: backward compat (−0.00% mass balance), extreme contrast (fresh vs near-halite: +194% dissolution difference confirms salting-out), uniform control (−1% difference confirms broadcast)
Cross-compiler: g++-13.3 and g++-14.2

Context

Second of two follow-ups requested by the reviewer of #5097. Independent of the BRINE/fugacity gating fix (to be filed separately).

svenn-t · 2026-04-15T13:29:46Z

Thanks for your work.

Since SALINITY is a keyword from the commercial simulator, that expects a constant and not one value per PVT region, I'm against changing the definition of this keyword. I suggest making a new salinity keyword which sets salinity per pvt region and implement your changes using the new keyword instead.

bska · 2026-04-15T13:40:50Z

I suggest making a new salinity keyword which sets salinity per pvt region and implement your changes using the new keyword instead.

We might consider implementing the SALINITR keyword. This doesn't precisely fit the request as that keyword has one value for each equation-of-state region (EOSNUM) instead of PVT region (PVTNUM), but maybe we could make it work per PVT region as well?

svenn-t · 2026-04-15T13:51:13Z

I suggest making a new salinity keyword which sets salinity per pvt region and implement your changes using the new keyword instead.

We might consider implementing the SALINITR keyword. This doesn't precisely fit the request as that keyword has one value for each equation-of-state region (EOSNUM) instead of PVT region (PVTNUM), but maybe we could make it work per PVT region as well?

Yes, that is a good suggestion. If an implementation based on EOSNUM does not give issues elsewhere (restarts?), I would do it that way.

dlmorenob · 2026-04-16T02:04:25Z

Thanks for the direction — SALINITR is OK. However, It's not in OPM's parser yet so this would be an addition, right?

I'll check the implementation using EOSNUM as you suggest, that said, I have not seen CO2STORE decks that set EOSNUM. Should SALINITR require EOSNUM in the deck, or default to PVTNUM when EOSNUM is absent?

dlmorenob · 2026-04-17T11:40:41Z

Following up on EOSNUM for SALINITR: the OPM Flow manual notes that EOSNUM is currently "parsed but its data ignored." Implementing SALINITR per EOSNUM would therefore require first wiring EOSNUM?
Is PVTNUM-based SALINITR still acceptable as an interim, or do you propose something else?

svenn-t · 2026-04-17T13:36:46Z

Following up on EOSNUM for SALINITR: the OPM Flow manual notes that EOSNUM is currently "parsed but its data ignored." Implementing SALINITR per EOSNUM would therefore require first wiring EOSNUM? Is PVTNUM-based SALINITR still acceptable as an interim, or do you propose something else?

Yes, SALINITR might be difficult to implement due to the current state of EOSNUM. I propose then to make an entirely new keyword (not named SALINITR, since that is also defined in the commercial simulator) and implement PVTNUM-based salinity.

Introduces SALINITP, an OPM-specific keyword sized by NTPVT, allowing different brine salinities per PVT region in CO2STORE/H2STORE decks. Avoids using a commercial keyword name (e.g. SALINITR) since EOSNUM plumbing is not yet supported in OPM. Co2StoreConfig now stores salt as std::vector<double>; the existing scalar salinity() accessor returns salt[0] (or 0.0 if unset). A new salinity(regionIdx) accessor reads per-region values, broadcasting single-record decks (SALINITY/SALTMF) to any region for backward compatibility. BrineCo2Pvt, Co2GasPvt and BlackOilFluidSystem now consult salinity(regionIdx) in their per-region init loops, fixing a silent bug where multi-region PVT tables and brine component molar masses used only the first region's salt. SALINITP lives under 900_OPM (not Eclipse 300) and prohibits SALINITY, SALTMF and BRINE; SALINITY continues to behave as a single-scalar broadcast and remains accepted for compatibility.

dlmorenob · 2026-04-29T00:38:02Z

Reworked per discussion: I introduce SALINITP under 900_OPM/S/, sized by NTPVT, prohibits SALINITY/SALTMF/BRINE. Existing scalar SALINITY/SALTMF unchanged. Per-region indexing wired through BrineCo2Pvt, Co2GasPvt, and the existing CO2STORE-guarded init in BlackOilFluidSystem. New tests for per-region behavior, scalar broadcast, and parser rejection of conflicting keywords.

svenn-t

I have some minor comments, mostly related to enforcing the salt vector to be NTPVT size regardless of keyword.

svenn-t · 2026-05-05T07:58:30Z

+        else if (props_section.hasKeyword<ParserKeywords::SALINITY>()) {
            const auto& molality = deck["SALINITY"].back().getRecord(0).getItem("MOLALITY").get<double>(0);
-            salt = 1.0 / (1.0 + 1.0 / (molality * MmNaCl));
+            salt.resize(1);


Resize according to NTPVT from TABDIMS and fill vector with the one value from SALINITY.

svenn-t · 2026-05-05T07:58:49Z

        else if (props_section.hasKeyword<ParserKeywords::SALTMF>()) {
            const auto& mole_frac = deck["SALTMF"].back().getRecord(0).getItem("MOLE_FRACTION").get<double>(0);
-            salt = mole_frac * MmNaCl / (mole_frac * (MmNaCl - MmH2O) + MmH2O);
+            salt.resize(1);


Resize according to NTPVT from TABDIMS and fill vector with the one value from SALTMF.

svenn-t · 2026-05-05T07:59:13Z

-        if (props_section.hasKeyword<ParserKeywords::SALINITY>()) {
+        // SALINITP / SALINITY / SALTMF convert to mass fraction.
+        // SALINITP (OPM-specific): per-PVT-region salinity, sized by NTPVT.
+        // SALINITY (commercial Eclipse): single scalar value, broadcast to all regions.


Remove "Eclipse"

svenn-t · 2026-05-05T08:02:26Z

    double Co2StoreConfig::salinity() const {
-        return salt;
+        return salt.empty() ? 0.0 : salt[0];
+    }
+


Remove salinity() function without any input. We will only use the version with regionIdx input.

svenn-t · 2026-05-05T08:03:45Z

+        if (salt.empty())
+            return 0.0;
+        // Broadcast single-record (SALINITY/SALTMF) value to all regions.
+        if (regionIdx >= salt.size())


Remove broadcasting. SALINITY and SALTMF should be the same size as NTPVT.

svenn-t · 2026-05-05T08:04:54Z

+        return salt[regionIdx];
+    }
+
+    std::size_t Co2StoreConfig::numSalinityRegions() const {


Remove. We can use NTPVT information if necessary.

dlmorenob force-pushed the fix/co2store-per-region-salinity branch from b970e94 to 4dbe890 Compare April 29, 2026 00:36

svenn-t added the manual:new-feature This is a new feature and should be described in the manual label Apr 29, 2026

svenn-t requested changes May 5, 2026

View reviewed changes

Conversation

dlmorenob commented Apr 12, 2026

Summary

Changes

Backward compatibility

Testing

Context

Uh oh!

svenn-t commented Apr 15, 2026

Uh oh!

bska commented Apr 15, 2026

Uh oh!

svenn-t commented Apr 15, 2026

Uh oh!

dlmorenob commented Apr 16, 2026

Uh oh!

dlmorenob commented Apr 17, 2026

Uh oh!

svenn-t commented Apr 17, 2026

Uh oh!

dlmorenob commented Apr 29, 2026

Uh oh!

svenn-t left a comment

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

svenn-t May 5, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants