Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: use mass and energy consistently for single phase solvers #3485

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

paveltomin
Copy link
Contributor

@paveltomin paveltomin commented Dec 8, 2024

unified where possible
for poromechanics - hard to do because porosity is being updated inside the assembly kernel, so left it as is
fixed glitch with double porosity update in poromechanics when it was updated both in flow solver and in poromechanics kernel using different equations leading to potential inconsistencies

@paveltomin paveltomin changed the title reafactor: use mass and energy consistently for single phase solvers refactor: use mass and energy consistently for single phase solvers Dec 9, 2024
@@ -241,46 +241,38 @@ DECLARE_FIELD( temperatureScalingFactor,
NO_WRITE,
"Scaling factors for temperature" );

DECLARE_FIELD( mass,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

mass went to single phase file since it is used there, not for compositional

@@ -594,8 +634,6 @@ void SinglePhaseBase::initializeFluidState( MeshLevel & mesh, arrayView1d< strin

// 2. save the initial density (for use in the single-phase poromechanics solver to compute the deltaBodyForce)
fluid.initializeState();

updateMass( subRegion );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this call is redundant, updateMass already called in updateFluidState few lines above

@@ -291,13 +260,11 @@ class SurfaceElementAccumulationKernel : public AccumulationKernel< SurfaceEleme
void computeAccumulation( localIndex const ei,
Base::StackVariables & stack ) const
{
Base::computeAccumulation( ei, stack, [&] ()
Base::computeAccumulation( ei, stack );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

keep it simple

stack.dSolidEnergy_dPres = dSolidVolume_dPres * m_rockInternalEnergy[ei][0];
stack.dSolidEnergy_dTemp = solidVolume * m_dRockInternalEnergy_dTemp[ei][0] + dSolidVolume_dTemp * m_rockInternalEnergy[ei][0];
}
{};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i could not remove this, somehow compiler gets always confused

@paveltomin paveltomin self-assigned this Dec 9, 2024
@paveltomin paveltomin added type: cleanup / refactor Non-functional change (NFC) flag: ready for review ci: run integrated tests Allows to run the integrated tests in GEOS CI labels Dec 9, 2024
@paveltomin paveltomin marked this pull request as ready for review December 9, 2024 18:03
@CusiniM
Copy link
Collaborator

CusiniM commented Dec 12, 2024

you may want to keep an eye on this: #3460

@paveltomin
Copy link
Contributor Author

you may want to keep an eye on this: #3460

Yes thanks

Copy link
Collaborator

@CusiniM CusiniM left a comment

Choose a reason for hiding this comment

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

LGTM

@paveltomin paveltomin changed the title refactor: use mass and energy consistently for single phase solvers fix: use mass and energy consistently for single phase solvers Dec 12, 2024
@paveltomin paveltomin added the type: bug Something isn't working label Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci: run integrated tests Allows to run the integrated tests in GEOS CI flag: ready for review type: bug Something isn't working type: cleanup / refactor Non-functional change (NFC)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants