Skip to content

feat: Change molarity concentration to molality concentration - #3996

Merged
victorapm merged 22 commits into
developfrom
feat/frankfei/molalityConcentration
Sep 21, 2026
Merged

victorapm merged 22 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 solventMassFraction as an optional input on the reactive fluid model (default 1, validated to lie in (0, 1]). HPCReact is molality-based, so species amounts are converted with molarity = molality * solventMassFraction * fluidDensity, where the density comes from the single fluid model and so already carries the pressure and temperature dependence. For now the fraction is prescribed and kept constant. Ideally it follows the chemical concentration, which thus needs species molar masses. Where that update would be computed and launched (GEOS or HPCReact) needs more discussion and deserves another PR.

  • Sourced the solventMassFraction 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 and added 1D serial chain reaction into the integrated test set

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 thread src/coreComponents/constitutive/fluid/reactivefluid/ReactiveSinglePhaseFluid.cpp Outdated

@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

@frankfeifan

Copy link
Copy Markdown
Contributor Author

@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

Thanks, @OmarDuran! Yes, you're right. My changes for now considers a constant ρs for simplicity, as the main goal is to ensure we convert the concentration to the right unit fed into HPCReact. That said, I took a look of the patch, and I think it's a genius idea and exactly aligned with my TODO in the code. Eventually, we may also want to allow this solventMassFraction to vary based on the dissolved species mass, but that can be done in a future PR. This is a good intermediate step and not that complicated at this moment, so I've applied this. Thank you very much!

@victorapm
victorapm merged commit 83ccd40 into develop Sep 21, 2026
22 checks passed
@victorapm
victorapm deleted the feat/frankfei/molalityConcentration branch September 21, 2026 14:15
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants