Skip to content

feat: Change molarity concentration to molality concentration - #3996

Open
frankfeifan wants to merge 19 commits into
developfrom
feat/frankfei/molalityConcentration
Open

frankfeifan wants to merge 19 commits into
developfrom
feat/frankfei/molalityConcentration

Conversation

@frankfeifan

@frankfeifan frankfeifan commented Mar 14, 2026

Copy link
Copy Markdown
Contributor

This PR changes convention of the concentration fed into HPCReact from molarity [mol/m3 of solution] or [mol/L of solution] to molality [mol/kg of solvent] using a factor called solventMassPerSolutionVolume (not solventDensity anymore) to be consistent with convention.

This is a preparatory step for introducing the activity model. So now, HPCReact is treated as a strictly molality-based library, and every conversion to a volumetric basis happens on the GEOS side through a single factor.

Specific changes include:

  • Switched the concentration passed to HPCReact from molarity to molality, converting back at every caller: accumulation, advective and diffusive flux, Dirichlet and source terms, and their thermal counterparts.

  • Introduced solventMassPerSolutionVolume [kg/m3] as an optional input on the reactive fluid model (default 1000) to serve as that conversion factor. For now, it is prescribed and kept constant, but ideally, we want to make it a function of P, T, and chemical composition. Specific implementation of methods and where these updates will be launched (GEOS or HPCReact) needs more discussions and deserves another PR.

  • Sourced the solventMassPerSolutionVolume from the solver's single reactive fluid model rather than a per-subregion scan, and turned the previously silent single-fluid-model assumption into an explicit input error, ensuring a single fluid model defined across all regions for a uniform dof layout.

  • Restored the finite-difference Jacobian test to dt = 1.0, since the reduction to 0.01 was a workaround for the inflated reaction term.

  • Rebaseline

Note: No HPCReact change is required, as the conversion factor lives in GEOS now tentatively.

@frankfeifan frankfeifan added the flag: requires rebaseline Requires rebaseline branch in integratedTests label Mar 17, 2026

@jafranc jafranc 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 👍

@tjb-ltk tjb-ltk 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.

A future discussion could consider posing this as multicomponent single phase option where specializations could be this could be this option or something like tracers .. not sure about the implications when starting with a num component generalization...

@frankfeifan

Copy link
Copy Markdown
Contributor Author

A future discussion could consider posing this as multicomponent single phase option where specializations could be this could be this option or something like tracers .. not sure about the implications when starting with a num component generalization...

Yeah. That's a good point. We may need a higher-level generic solver base for such multi-component single phase option (can inherit from SinglePhaseBase as I did for this reactive transport solver).

Comment on lines 33 to +36
using namespace hpcReact::bulkGeneric;
using namespace hpcReact::geochemistry;
using namespace hpcReact::ChainGeneric;
using namespace hpcReact::MoMasBenchmark;

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.

Double check if we actually need this.

Suggested change
using namespace hpcReact::bulkGeneric;
using namespace hpcReact::geochemistry;
using namespace hpcReact::ChainGeneric;
using namespace hpcReact::MoMasBenchmark;

@OmarDuran OmarDuran 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.

@frankfeifan thank you for taking care of computing actual mass balance.

I might be mistaken, but only with constant ρs, the conversions are fully correct. I’m unsure whether they remain consistent for a compressible fluid.

I also refactored a few files to accommodate these changes. See the patch below; if it makes sense, feel free to apply it.

molality_solventMassFraction.patch

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

Labels

changes XML input ci: run device builds Allows to run the device (CUDA/HIP) builds 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 flag: requires updated submodule(s)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants