[GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations #12361

rfaasse · 2024-05-08T12:01:32Z

📝 Description
This PR changes the signature of the CalculateStrain and CalculateCauchyStrain functions, so they no longer use ElementVariables for their in-/outputs. It introduces a bit of duplications, but that will be removed when we create lists for these inputs in later PRs. It was decided to create this PR now, since it is very narrowly scoped and therefore (hopefully) easy to review

…Strain` to return a vector and takes relevant inputs instead of variables

…rn a vector and takes relevant inputs instead of variables

…) to return a vector instead of using the ElementVariables

…rmation gradient, instead of getting it from ElementVariables

…deformation gradient, instead of getting it from ElementVariables

…train-vector-calculations

…trix, displacements and UseHenckyStrainFlag instead of using ElementVariables

…b matrix, displacements and UseHenckyStrainFlag instead of using ElementVariables

…train-vector-calculations

avdg81

I'm very satisfied with this clean-up action. In particular, I like that you have improved the function signatures (adding a proper return type and using specific function parameters rather than the ElementVariables container). Thank you for making these changes.

I have a few minor suggestions, none of them are blocking.

avdg81 · 2024-05-08T14:21:53Z

applications/GeoMechanicsApplication/custom_elements/U_Pw_small_strain_element.cpp

+    if (UseHenckyStrain) {
+        return StressStrainUtilities::CalculateHenckyStrain(rDeformationGradient, VoigtSize);
    } else {
-        this->CalculateCauchyStrain(rVariables);
+        return this->CalculateCauchyStrain(rB, rDisplacements);
    }
 }


We could simplify this even further by either using the ternary operator or by removing the explicit else keyword:

return UseHenckyStrain ? StressStrainUtilities::CalculateHenckyStrain(rDeformationGradient, VoigtSize) : this->CalculateCauchyStrain(rB, rDisplacements);

Or, if that is too compact, use this:

if (UseHenckyStrain) { return StressStrainUtilities::CalculateHenckyStrain(rDeformationGradient, VoigtSize); } return this->CalculateCauchyStrain(rB, rDisplacements);

Agreed, makes it cleaner!

avdg81 · 2024-05-08T14:29:12Z

applications/GeoMechanicsApplication/custom_elements/U_Pw_small_strain_element.hpp

+    virtual Vector CalculateCauchyStrain(const Matrix& rB, const Vector& rDisplacements);
+    virtual Vector CalculateStrain(const Matrix& rDeformationGradient,
+                                   const Matrix& rB,
+                                   const Vector& rDisplacements,
+                                   bool          UseHenckyStrain);


I would suggest to make these member functions const, to express that they won't modify the element's state.

avdg81 · 2024-05-08T14:56:04Z

applications/GeoMechanicsApplication/custom_elements/small_strain_U_Pw_diff_order_element.cpp

    } else {
-        this->CalculateCauchyStrain(rVariables);
+        return this->CalculateCauchyStrain(rB, rDisplacements);
    }
 }


As for the case of the small strain U-Pw element, I would remove the explicit else here.

avdg81 · 2024-05-08T14:57:00Z

applications/GeoMechanicsApplication/custom_elements/small_strain_U_Pw_diff_order_element.hpp

+    virtual Vector CalculateCauchyStrain(const Matrix& rB, const Vector& rDisplacements);
+    virtual Vector CalculateStrain(const Matrix& rDeformationGradient,
+                                   const Matrix& rB,
+                                   const Vector& rDisplacements,
+                                   bool          UseHenckyStrain);


I would like to suggest to make these member functions const.

markelov208

Richard, thank you for this PR, it became more visible what should be defined and removed unnecessary actions. I have only general questions.

markelov208 · 2024-05-08T20:50:38Z

applications/GeoMechanicsApplication/custom_elements/U_Pw_small_strain_FIC_element.cpp

@@ -144,7 +144,10 @@ void UPwSmallStrainFICElement<TDim, TNumNodes>::InitializeNonLinearIteration(con
        Variables.B = b_matrices[GPoint];

        // Compute infinitessimal strain
-        this->CalculateStrain(Variables, GPoint);
+        Variables.F            = this->CalculateDeformationGradient(GPoint);
+        Variables.detF         = MathUtils<>::Det(Variables.F);


I was wondering, putting this line into SetConstitutiveParameters function violates the title of this function? If not, then some duplications will go out now.
A general question is why some CalculateOnIntegrationPoints function call SetConstitutiveParameters when others do not?

Indeed we add a bit of extra duplication in this PR, it's a bit of a trade-off with limiting the use of the ElementVariables container. Since the Variables.F is used in the strain vector calculation and passed on to the constitutive parameters, I think it's better to keep it explicit like it is now.

In a next PR, which is now still in draft status, lists are extracted for F, detF and the strain vectors (see https://github.com/KratosMultiphysics/Kratos/pull/12363/files#diff-5a457c1c0c5d61a1ff97989e636c83f79506ea59254d16659303b48efde6f3cbR318-R333). I think that state is acceptable, especially since it becomes clearer and clearer who uses what (instead of everyone grabbing anything out of the ElementVariables bucket). I'm curious to know your opinion on this!

As for the question of when do/don't we use SetConstitutiveParameters: I think that depends on what we are actually calculating on the integration points. E.g. when just calculating the StrainVector, we don't need to use the constitutive law. However, if we want to convert that strain to a stress vector and generate that as an output, we do need to set the constitutive parameters.

Hi Richard, thank you for the info. My message is not clear, probably, because it was done in the late evening. ;) My comment was on moving Variables.detF = MathUtils<>::Det(Variables.F); to SetConstitutiveParameters . It is clear that everything will change soon.
Concerning the next PR, it seems these functions are the same on the first glance. If it is true then they should be in utilities.

WPK4FEM

This is a clear step towards getting rid of the bucket Variables.
The apparent increased line count ( caused by the explicit initialization of Variables.F, Variables.DetF etc. will disappear by the time that the bucket Variables is removed.

Your action also makes clear that the strain computation is a general continuum strain and not very element specific here.

WPK4FEM · 2024-05-10T06:43:08Z

applications/GeoMechanicsApplication/custom_elements/U_Pw_small_strain_element.cpp

+Vector UPwSmallStrainElement<TDim, TNumNodes>::CalculateStrain(const Matrix& rDeformationGradient,
+                                                               const Matrix& rB,
+                                                               const Vector& rDisplacements,
+                                                               bool          UseHenckyStrain)
 {
-    if (rVariables.UseHenckyStrain) {
-        rVariables.F    = this->CalculateDeformationGradient(GPoint);
-        rVariables.detF = MathUtils<>::Det(rVariables.F);
-        noalias(rVariables.StrainVector) = StressStrainUtilities::CalculateHenckyStrain(rVariables.F, VoigtSize);
+    if (UseHenckyStrain) {
+        return StressStrainUtilities::CalculateHenckyStrain(rDeformationGradient, VoigtSize);
    } else {
-        this->CalculateCauchyStrain(rVariables);
+        return this->CalculateCauchyStrain(rB, rDisplacements);
    }
 }

 template <unsigned int TDim, unsigned int TNumNodes>
-void UPwSmallStrainElement<TDim, TNumNodes>::CalculateCauchyStrain(ElementVariables& rVariables)
+Vector UPwSmallStrainElement<TDim, TNumNodes>::CalculateCauchyStrain(const Matrix& rB, const Vector& rDisplacements)
 {
-    noalias(rVariables.StrainVector) = prod(rVariables.B, rVariables.DisplacementVector);
+    return prod(rB, rDisplacements);
 }


With the argument lists showing precisely what is being used, it also becomes clear that also CalculateCauchyStrain is not element specific. It is always B * u and can be done in the stress strain utilities.
When VoigtSize is an argument of CalculateStrain, it can also be moved to the stress strain utilities.

What I did not check is whether the current member functions are part of the element base class.

Good point, we discussed this and agreed to leave these functions in place for this PR, but agreed that since this does not have any element-specific behavior, it should move to the stress strain utils (of course with UTs).

WPK4FEM

Good to go.

rfaasse added 9 commits May 7, 2024 11:11

Changed signature of `SmallStrainUPwDiffOrderElement::CalculateCauchy…

7061c42

…Strain` to return a vector and takes relevant inputs instead of variables

Changed signature of UPwSmallStrainElement::CalculateStrain to retu…

5dc686a

…rn a vector and takes relevant inputs instead of variables

Changed signature of CalculateStrain (diff order and non-diff order…

ce511b3

…) to return a vector instead of using the ElementVariables

Changed signature of CalculateStrain (diff order) to input the defo…

63bd1a6

…rmation gradient, instead of getting it from ElementVariables

Actually use deformation gradient that was passed on via arguments

5f451f1

Changed signature of CalculateStrain (non-diff order) to input the …

5827e2c

…deformation gradient, instead of getting it from ElementVariables

Merge remote-tracking branch 'origin/master' into geo/12319-extract-s…

58b5bd6

…train-vector-calculations

Changed signature of CalculateStrain (diff order) to input the b ma…

05fd57f

…trix, displacements and UseHenckyStrainFlag instead of using ElementVariables

Changed signature of CalculateStrain (non-diff order) to input the …

7f65441

…b matrix, displacements and UseHenckyStrainFlag instead of using ElementVariables

rfaasse linked an issue May 8, 2024 that may be closed by this pull request

[GeoMechanicsApplication] Split the calculation of the inputs for the utility functions for element matrices and the calling of the functions #12319

Closed

Removed some redundant calculations of the determinant of F

dcc3bac

rfaasse marked this pull request as ready for review May 8, 2024 12:18

rfaasse requested review from WPK4FEM, avdg81 and markelov208 May 8, 2024 12:18

Merge remote-tracking branch 'origin/master' into geo/12319-extract-s…

a34346b

…train-vector-calculations

avdg81 reviewed May 8, 2024

View reviewed changes

rfaasse changed the title ~~Geo/12319 extract strain vector calculations~~ [GeoMechanicsApplication] Extract strain vector calculations May 8, 2024

markelov208 reviewed May 8, 2024

View reviewed changes

WPK4FEM reviewed May 10, 2024

View reviewed changes

Processed review comments

96add5c

rfaasse requested review from avdg81, WPK4FEM and markelov208 May 10, 2024 11:02

WPK4FEM approved these changes May 10, 2024

View reviewed changes

markelov208 approved these changes May 10, 2024

View reviewed changes

rfaasse changed the title ~~[GeoMechanicsApplication] Extract strain vector calculations~~ [GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations May 10, 2024

rfaasse merged commit dd52469 into master May 10, 2024
11 checks passed

rfaasse deleted the geo/12319-extract-strain-vector-calculations branch May 10, 2024 11:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations #12361

[GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations #12361

rfaasse commented May 8, 2024 •

edited

Loading

avdg81 left a comment

avdg81 May 8, 2024

rfaasse May 10, 2024

avdg81 May 8, 2024

rfaasse May 10, 2024

avdg81 May 8, 2024

rfaasse May 10, 2024

avdg81 May 8, 2024

rfaasse May 10, 2024

markelov208 left a comment

markelov208 May 8, 2024 •

edited

Loading

rfaasse May 10, 2024

markelov208 May 10, 2024

WPK4FEM left a comment

WPK4FEM May 10, 2024

rfaasse May 10, 2024

WPK4FEM left a comment

[GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations #12361

[GeoMechanicsApplication] Reduce use of ElementVariables in element strain vector calculations #12361

Conversation

rfaasse commented May 8, 2024 • edited Loading

avdg81 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

markelov208 left a comment

Choose a reason for hiding this comment

markelov208 May 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WPK4FEM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WPK4FEM left a comment

Choose a reason for hiding this comment

rfaasse commented May 8, 2024 •

edited

Loading

markelov208 May 8, 2024 •

edited

Loading