Skip to content

Conversation

@francoishamon
Copy link
Contributor

In the CO2-brine multifluid package, we currently compute the phase internal energy u_p using a very crude formula here (same for both phases):

 u_p =  0.001 * pres + 1.0 * temp;

with which we cannot match published internal energy values for CO2 and brine phases and cannot produce valid temperature fronts in the thermal solver.

I propose to change this to the standard formula (used in other simulators)

u_p = h_p - pres / \rho_p

where h_p is the phase enthalpy, and \rho_p is the phase mass density. With this formula, we get a good match with published internal energy values for CO2 and brine.

Since this formula is general and can be used by other fluid models (including CompositionalMultiphaseFluid at some point), I moved the computation of internal energy to MultiFluidBase, and removed the INT_ENERGY template from the CO2BrineFluid class.

No need for rebaseline, this only affects the results in #1225

"ViscosityFun",
"EnthalpyFun",
"InternalEnergyFun" );
"EnthalpyFun" );
Copy link
Contributor

Choose a reason for hiding this comment

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

I would encourage you to build the same tests that I did in #1778 so future refactorings will be safer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, I missed these in the PR where they were introduced. We definitely shouldn't be redeclaring these strings 4 times - the enumeration type must be made non-dependent on template parameters of CO2BrineFluid in one of the following ways:

  • moving the enum definition out of class into the namespace geosx::constitutive (I'd prefer not to)
  • making a non-template class CO2BrineFluidBase containing the enum and everything else that does not depend on the template parameters
  • or moving the enum definition entirely into .cpp file - it is not used anywhere else. Or maybe get rid if enum entirely? It's not actually used anywhere, except for getting strings (once per value). Or at least it needs to be used as an enum - by converting the input string to it, then switching on the value, instead of branching on string comparisons.

This is a future TODO item, not something to address in current PR, which is ready to be merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok sounds good, I did not know. I am going to merge this PR now since I realize now that it is at the top of the queue, and I will remove these enums immediately in the main thermal PR. Thanks!

@francoishamon francoishamon merged commit be12179 into develop Mar 30, 2022
@francoishamon francoishamon deleted the refactor/hamon/internalEnergy branch March 30, 2022 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants