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

Fluid flux output computation similar to the one in CalculateRHS. #13159

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

WPK4FEM
Copy link
Contributor

@WPK4FEM WPK4FEM commented Feb 21, 2025

📝 Description
The output item FLUID_FLUX_VECTOR did not take the Bishop coefficient for partial saturation into account. This has been corrected, such that now the flux computation is the same as when forming the RHS.

@WPK4FEM WPK4FEM self-assigned this Feb 21, 2025
markelov208
markelov208 previously approved these changes Feb 21, 2025
Copy link
Contributor

@markelov208 markelov208 left a comment

Choose a reason for hiding this comment

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

Hi Wijtze-Pieter, thank you for fixing the difference between calculation and output. I have a few non-blocking comments.

…de English error text, renamed variable to be consistent.
Copy link
Contributor

@rfaasse rfaasse left a comment

Choose a reason for hiding this comment

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

Nice fix for a hard-to-catch subtle issue. I don't have any blocking comments, but if possible, I'd very much like it if we could add some unit tests to make sure we don't end up in a similar situation.


auto relative_permeability_values = this->CalculateRelativePermeabilityValues(
GeoTransportEquationUtilities::CalculateFluidPressures(Variables.NContainer, Variables.PressureVector));
const auto fluid_pressures = GeoTransportEquationUtilities::CalculateFluidPressures(
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to unit-test this behavior?

auto relative_permeability_values =
CalculateRelativePermeabilityValues(GeoTransportEquationUtilities::CalculateFluidPressures(
Variables.NpContainer, Variables.PressureVector));
const auto fluid_pressures = GeoTransportEquationUtilities::CalculateFluidPressures(
Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch, I fear this was introduced in the refactor efforts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants