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

[GeoMechanicsApplication] Simplified fluid flux for pw element #12338

Merged
merged 8 commits into from
May 6, 2024

Conversation

rfaasse
Copy link
Contributor

@rfaasse rfaasse commented May 2, 2024

📝 Description
We found that to calculate the fluid flux variable, the permeability update factor was calculated using strains, regardless of Pw vs UPw. Since strains are meaningless for Pw elements, this needed to be rectified. Therefore, a function is created to calculate fluid fluxes, which takes as an input a list of permeability update factors for the integration points (which are now always 1 for the Pw elements and calculated via the strain for UPw elements).

@rfaasse rfaasse changed the title Geo/12319 simplified fluid flux for pw element [GeoMechanicsApplication] Simplified fluid flux for pw element May 3, 2024
@rfaasse rfaasse self-assigned this May 3, 2024
@rfaasse rfaasse added Refactor When code is moved or rewrote keeping the same behavior GeoMechanics Issues related to the GeoMechanicsApplication labels May 3, 2024
@rfaasse rfaasse marked this pull request as ready for review May 3, 2024 12:45
@rfaasse rfaasse requested review from WPK4FEM and avdg81 May 3, 2024 12:45
WPK4FEM
WPK4FEM previously approved these changes May 3, 2024
Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

Hi Richard,
The intent is clear to me.
Only some nagging about layout.
Wijtze Pieter

mRetentionLawVector[GPoint]->CalculateRelativePermeability(RetentionParameters);

noalias(GradPressureTerm) = prod(trans(Variables.GradNpT), Variables.PressureVector);

Copy link
Contributor

Choose a reason for hiding this comment

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

lines 1124 and 1126 are constructing the contents of the same variable. This interspacing contradicts that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed blank line

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

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

Nice improvement of the code. I have only one very minor comment. Let's try to get this in quickly, so you can proceed with the PR that started all this (extracting lists of $B$ matrices).

@@ -1112,6 +1091,48 @@ void UPwSmallStrainElement<TDim, TNumNodes>::InitializeBiotCoefficients(ElementV
KRATOS_CATCH("")
}

template <unsigned int TDim, unsigned int TNumNodes>
std::vector<array_1d<double, TDim>> UPwSmallStrainElement<TDim, TNumNodes>::CalculateFluidFluxes(
const std::vector<double>& permeability_update_factors, const ProcessInfo& rCurrentProcessInfo)
Copy link
Contributor

Choose a reason for hiding this comment

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

To be in line with the Kratos Style Guide, we should rename the first parameter:

Suggested change
const std::vector<double>& permeability_update_factors, const ProcessInfo& rCurrentProcessInfo)
const std::vector<double>& rPermeabilityUpdateFactors, const ProcessInfo& rCurrentProcessInfo)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@rfaasse rfaasse requested review from avdg81 and WPK4FEM May 6, 2024 06:38
@rfaasse rfaasse enabled auto-merge (squash) May 6, 2024 07:01
Copy link
Contributor

@WPK4FEM WPK4FEM left a comment

Choose a reason for hiding this comment

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

Resize kijkt zelf wel naar de oorspronkelijke grootte, daar hoeft geen test voor en hoeft ook niet herhaald te worden met dezelfde grootte met een andere naam.
Dat kan best in een volgende ronde, want we zijn hier toch nog uitgebreid bezig.

@rfaasse rfaasse merged commit 1b78e7e into master May 6, 2024
11 checks passed
@rfaasse rfaasse deleted the geo/12319_simplified_fluid_flux_for_Pw_element branch May 6, 2024 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GeoMechanics Issues related to the GeoMechanicsApplication Refactor When code is moved or rewrote keeping the same behavior
Projects
None yet
3 participants