Skip to content

feat: Move CO2-Brine parameters to xml#3777

Open
dkachuma wants to merge 25 commits into
developfrom
dkachuma/co2-brine-to-xml
Open

feat: Move CO2-Brine parameters to xml#3777
dkachuma wants to merge 25 commits into
developfrom
dkachuma/co2-brine-to-xml

Conversation

@dkachuma

@dkachuma dkachuma commented Aug 15, 2025

Copy link
Copy Markdown
Contributor

This PR removes the parameter files used for CO2-Brine model input. This is replaced by direct xml entries. This is a breaking change in terms of user interaction requiring changes to input models. This is evidenced by the extensive changes required in the geos integrated tests.

The key change is the removal of the two fields phasePVTParaFiles and flashModelParaFile on the CO2BrineFluid models. The information that was in these files is now required within the xml itself. This includes:

  • Pressure and temperature table ranges. Generally these models involve creation of tables internally and the user was expected to enter the points for these tables by specifying a min, max and step for pressure and temperature. Now the pressureCoordinates captures the range while the pressureInterval captures the step and similarly for temperature. The new formulation is different from the old in that
    • All properties have the same pressure and temperature discretisation points whereas the old setup allowed different points for the different properties (flash, density, viscosity).
    • The new setup does not assume equally spaced points. The user can explicitly give the list of pressures in pressureCoordinates and ignore pressureInterval. This is useful in some cases where a log space might be more appropriate.
  • The salinity with a default of 0.
  • The density and viscosity Ezrokhi coefficients. These can be captured with ezrokhiDensityCoefficients and ezrokhiViscosityCoefficients. These default to 0 if not specified.
  • The solubility model can now be explicitly defined with choices between (DuanSun (default), SpycherPruess and Tables). The table formulation works in conjunction with the solubilityTables field in which the user provides (externally generated) solubility tables directly.

The implementation involves creation of the BrineFluidParameters object to contain all the parameters. This is then passed around when creating PVT models instead of lists of strings.

The CO2-Brine documentation has been updated to reflect not only the changes but also to include all previously developed features that had not been documented.

Changes in results:

  • The biggest change is the use of common points for all tables. Some of the examples were using different table ranges for the different properties. This has resulted in reasonable changes in the results now that a common set of points and ranges is used.

Example change:

    <CO2BrineEzrokhiFluid
      name="fluid"
      phaseNames="{ gas, water }"
      componentNames="{ co2, water }"
      componentMolarWeight="{ 44e-3, 18e-3 }"
      phasePVTParaFiles="{ tables/pvtgas.txt, tables/pvtliquid_ez.txt }"
      flashModelParaFile="tables/co2flash.txt" />
    <CO2BrineEzrokhiFluid
      name="fluid"
      phaseNames="{ gas, water }"
      componentNames="{ co2, water }"
      componentMolarWeight="{ 44e-3, 18e-3 }"
      pressureCoordinates="{1.0e5, 6e7}"
      pressureInterval="1e5"
      temperatureCoordinates="{283.15, 393.5}"
      temperatureInterval="5"
      salinity="1.901285269"
      ezrokhiDensityCoefficients="{0.1033, -2.2991e-5, -2.3658e-6}"
      ezrokhiViscosityCoefficients="{0, 0, 0}" />

Since this is a breaking change, detailed migration instructions are posted at #4022

@dkachuma dkachuma self-assigned this Aug 15, 2025
@dkachuma dkachuma added ci: run CUDA builds Allows to triggers (costly) CUDA jobs flag: requires rebaseline Requires rebaseline branch in integratedTests ci: run integrated tests Allows to run the integrated tests in GEOS CI ci: run code coverage enables running of the code coverage CI jobs labels Aug 15, 2025
@MelReyCG

MelReyCG commented Sep 16, 2025

Copy link
Copy Markdown
Contributor

This is really a good thing. This will also allow to simply unit tests.
Will the previous behaviour with external tables be kept for backward compatibility? Or is it possible to include a documentation like "How to convert PVT parameters files to the new PVT settings."?

@dkachuma

Copy link
Copy Markdown
Contributor Author

Will the previous behaviour with external tables be kept for backward compatibility?

No. This is a breaking change. The idea is to add an issue describing how to change the parameters.

@dkachuma dkachuma marked this pull request as ready for review April 8, 2026 02:00
@dkachuma dkachuma requested a review from cssherman as a code owner May 14, 2026 03:28
{
for( integer ic = 0; ic < 3; ic++ )
{
m_ezrokhiDensityCoefficients.emplace_back( 0.0 );

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.

I suggest we set default values here, and then remove these parameters from the integrated test XMLs unless they differ from the default. It looks like they all use the same set of values.

{
for( integer ic = 0; ic < 3; ic++ )
{
m_ezrokhiViscosityCoefficients.emplace_back( 0.0 );

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.

Ditto. Could set good default values. Users may not know how to set these params properly.

Comment on lines +80 to +81
ezrokhiDensityCoefficients="{0.1033, -2.2991e-5, -2.3658e-6}"
ezrokhiViscosityCoefficients="{0, 0, 0}" />

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.

Suggest we set default values and remove these coefficients wherever possible from XMLs.

InputError );

// Salinity must not be negative
GEOS_THROW_IF_LT_MSG( m_salinity, 0.0,

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.

I think this check could be handled by the BrineFluidParameters object itself. We could centralize all checking of parameter physicality there?


.. note::
It is the responsibility of the user to make sure that the pressure and temperature values encountered in the simulation (in the reservoir and in the well) are within the bounds specified in the PVT files. GEOS will not throw an error if a value outside these bounds is encountered, but the (nonlinear) behavior of the simulation and the quality of the results will likely be negatively impacted.
It is the responsibility of the user to make sure that the pressure and temperature values encountered in the simulation (in the reservoir and in the well) are within the bounds specified by the coordinate attributes in the XML. GEOS will not throw an error if a value outside these bounds is encountered, but the (nonlinear) behavior of the simulation and the quality of the results will likely be negatively impacted.

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.

Not specific to this PR, but this is such a common user error I wonder if we should revisit the idea that it is not our responsibility to check for out of bounds and warn the user. Would be nice if this was a flaggable check. We could probably even just check the initial formation pressure range and any BHP conditions flagged in the XML and say someone is likely to exceed bounds, without doing range checking at every table call mid-simulation.

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.

Or once a timestep compute the (max / min) (pressure / temp) over the whole domain and see if those values are in bounds for the table.

@joshua-white joshua-white 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.

Looks great. Just a few comments regarding providing default values and making sure we do good parameter consistency checking when possible.

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

Labels

ci: run code coverage enables running of the code coverage CI jobs ci: run CUDA builds Allows to triggers (costly) CUDA jobs ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review flag: requires rebaseline Requires rebaseline branch in integratedTests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants