[GeoMechanicsApplication] Implement Calculator-based workflow for Pw line element #12778

rfaasse · 2024-10-24T07:39:37Z

📝 Description
This PR implements a calculator-based workflow for the contributions added to the left hand side and right hand side in the Pw line element. In short, we now provide a list of contribution types to the element, which then creates the corresponding calculator and its input.

The advantage of this is that the actual calculation of the contributions is delegated to the calculators, meaning we can quite easily determine what contributions we would like in an element, without creating new elements in the class hierarchy.

If this workflow turns out to be satisfactory for the Pw line element, we can start taking steps to roll this out to our other (U)Pw elements as well as use it for the piping element.

…calculators-to-pw-line-element

…ents

…d coded 1D)

WPK4FEM

I like the general applicability of the idea presented here. Some aspects of the implementation I have trouble understanding. Please explain a bit further to me.

WPK4FEM · 2024-10-25T13:10:05Z

applications/GeoMechanicsApplication/custom_elements/calculation_contribution.h

+namespace Kratos
+{
+
+enum class CalculationContribution { Permeability, Compressibility };


To be extended with mass, damping, stiffness, UPwCoupling and PwUCoupling when working on the UPw elements.
A similar thought process can perhaps be followed for the geometrical nonlinear contributions

The GeometricStiffness matrix would be a very suitable one indeed, hadn't thought of that but when having a quick look, it seems we can probably remove the updated lagrangian elements from the hierarchy if we have a 'GeometricStiffnessCalculator' 🎉

For the mass and damping, I think it's a good idea to add the calculators, however since adding these terms is the responsibility of the scheme, this will require a bit more discussion.

WPK4FEM · 2024-10-25T13:12:32Z

applications/GeoMechanicsApplication/custom_elements/calculator.h

+
+    virtual Matrix                    LHSContribution()               = 0;
+    virtual Vector                    RHSContribution()               = 0;
+    virtual std::pair<Matrix, Vector> CalculateLeftAndRightHandSide() = 0;


On first sight this looks a bit like CalculateLocalSystem,
Maybe include Contribution in the name ( CalculateLeftAndRightHandSideContirbution of CalculateLocalSystemContribution ).

I renamed it to LocalSystemContribution, to be in line with the LHS/RHS names, let me know whether this is clear enough!

WPK4FEM · 2024-10-28T13:15:34Z

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

+
+Matrix CompressibilityCalculator::LHSContribution(const Matrix& rCompressibilityMatrix) const
+{
+    return rCompressibilityMatrix * mInputProvider.GetDtPressureCoefficient();


The DtPressureCoefficient is determined by the time integration scheme. I wonder if we should do this multipication inside the calculator of leave this outside to avoid the mingling of functionality in the calculator.

As discussed replaced it with the more general 'GetMatrixScalarFactor'

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

WPK4FEM · 2024-10-28T13:47:02Z

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

+    static auto GetDtPressureCoefficientLambda(const ProcessInfo& rCurrentProcessInfo)
+    {
+        return [&rCurrentProcessInfo]() { return rCurrentProcessInfo[DT_PRESSURE_COEFFICIENT]; };
+    }


This information belongs to the time integration scheme.

As we discussed, I'll change this to GetMatrixScalarFactor, to be filled in by the scheme via the current process info object passed to the element.

Let me know if this is better

WPK4FEM · 2024-10-28T13:47:55Z

applications/GeoMechanicsApplication/custom_utilities/element_utilities.cpp

+    if (Dimension == 1) {
+        BoundedMatrix<double, 1, 1> result_1d;
+        FillPermeabilityMatrix(result_1d, Prop);
+        return result_1d;
+    }
+    if (Dimension == 2) {
+        BoundedMatrix<double, 2, 2> result_2d;
+        FillPermeabilityMatrix(result_2d, Prop);
+        return result_2d;
+    }
+    if (Dimension == 3) {
+        BoundedMatrix<double, 3, 3> result_3d;
+        FillPermeabilityMatrix(result_3d, Prop);
+        return result_3d;
+    }
+
+    KRATOS_ERROR << "Dimension " << Dimension << " is not supported" << std::endl;
+}


Looks like a case statement.

I had some trouble with the switch for some reason, maybe @avdg81 can help me out next week with this

applications/GeoMechanicsApplication/tests/cpp_tests/custom_elements/test_pw_line_element.cpp

markelov208

Hi Richard, 'a small step to make major changes'.
I've got an impression that transientPwPipeElement is behind transientPwELement in terms of the functions' style. If this is true, shall it be another PR to make them consistent? Thank you.

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

avdg81

This is a very nice extension towards more generic handling of contributions to left hand side matrices and right hand side vectors. It will help us tremendously to reduce the explosion of element classes due to different combinations of features. Thank you so much for all the time and effort that you have put into this.
I have several suggestions, but overall this already looks quite good.

applications/GeoMechanicsApplication/custom_elements/calculator.h

applications/GeoMechanicsApplication/custom_elements/permeability_calculator.h

applications/GeoMechanicsApplication/custom_elements/permeability_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.h

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

…calculators-to-pw-line-element

rfaasse

I think I processed all comments, there might be a few improvements still possible, let's discuss next week!

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

rfaasse · 2024-11-01T16:03:07Z

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.h

+            : mGetElementProperties(std::move(rElementProperties)),
+              mGetRetentionLaws(std::move(rRetentionLaws)),
+              mGetNContainer(std::move(rNContainer)),
+              mGetIntegrationCoefficients(std::move(rIntegrationCoefficients)),
+              mGetDtPressureCoefficient(std::move(DtPressureCoefficient)),
+              mGetNodalValues(std::move(rNodalValuesOfDtWaterPressure))


Because the members are getter functions to be called when needed

applications/GeoMechanicsApplication/custom_elements/permeability_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

rfaasse · 2024-11-01T16:04:59Z

applications/GeoMechanicsApplication/custom_utilities/element_utilities.cpp

+    if (Dimension == 1) {
+        BoundedMatrix<double, 1, 1> result_1d;
+        FillPermeabilityMatrix(result_1d, Prop);
+        return result_1d;
+    }
+    if (Dimension == 2) {
+        BoundedMatrix<double, 2, 2> result_2d;
+        FillPermeabilityMatrix(result_2d, Prop);
+        return result_2d;
+    }
+    if (Dimension == 3) {
+        BoundedMatrix<double, 3, 3> result_3d;
+        FillPermeabilityMatrix(result_3d, Prop);
+        return result_3d;
+    }
+
+    KRATOS_ERROR << "Dimension " << Dimension << " is not supported" << std::endl;
+}


I had some trouble with the switch for some reason, maybe @avdg81 can help me out next week with this

applications/GeoMechanicsApplication/custom_elements/calculator.h

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

WPK4FEM

I'm happy with this.

WPK4FEM · 2024-11-04T14:57:51Z

applications/GeoMechanicsApplication/custom_elements/permeability_calculator.cpp

+        GeoElementUtilities::FillPermeabilityMatrix(r_properties, local_dimension);
+
+    const auto   number_of_nodes           = shape_function_gradients[0].size1();
+    auto         result                    = Matrix{ZeroMatrix{number_of_nodes, number_of_nodes}};


This dimension is correct, as there is one D.O.F. per node. The more natural dimension is the number of Pw D.O.F. multiplied with itself.

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

avdg81

This one is almost ready I think. I noticed there was one function renamed which you probably did not intend to... Please check whether my observation is correct. Apart from that I only have one more minor suggestion and a response to one of your questions.

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

…the order of a multiplication

WPK4FEM

This is a very scalable implementation that can be extended to our other elements.
Think it is ready to go.

markelov208

Hi Richard, thank you very much for the introduction of calculators.

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

rfaasse added 30 commits October 22, 2024 09:22

Added first unit test and made initialize function public

795259b

Put element under an angle to allow for fluid body vector

f5d7456

Added interface for the Calculator

7a2aa3f

modified test such that it is more sensitive to compressibility

4244fc0

Clean up some redundant stuff from the test

8ea34cc

Format test

da2d58f

Moved compressibility terms of both LHS and RHS to calculator

1472ace

Cleaned up compressibility functions in element

baa1ac8

Minor clean-up

3211586

Clean-up in calculator

58db729

Made lambdas re-usable

23153c9

Moved permeability calculation to calculator

85f0a87

Clean up

e82ece8

Renames for clarity

771a576

Removed redundant blank lines

5036387

Remove leftover functions which became redundant

fee9607

Shortening piece of code

b17f37a

Add missing include

facdcd5

Creating the calculators based on a list of enums

b97a323

Made it possible to change the number of contributions at registration

7807547

Minor clean-up + formatting

8ff26ed

Merge remote-tracking branch 'origin/master' into geo/add-extendible-…

d585719

…calculators-to-pw-line-element

Changed function to return matrix + vector instead of using the argum…

6c435f2

…ents

Moved enum to separate file + formatting

0b54a5d

Fix gcc build in pipeline

f6c457c

Made permeability calculator not dependent on local dimension (no har…

6071333

…d coded 1D)

Removing tests not related to the local system function

a411aa8

Cleanup and moving private & public functions around

db01e12

Fix error for non-void function not returning a value at every path

6999330

Fix typo in function

0416c10

rfaasse added the Refactor When code is moved or rewrote keeping the same behavior label Oct 25, 2024

Fixed three code smells

e5e2d50

rfaasse marked this pull request as ready for review October 25, 2024 07:53

WPK4FEM reviewed Oct 28, 2024

View reviewed changes

rfaasse requested review from avdg81 and markelov208 October 29, 2024 07:52

rfaasse mentioned this pull request Oct 29, 2024

[GeoMechanicsApplication] Investigation of transient/steady state policy for Pw elements #12762

Closed

markelov208 reviewed Nov 1, 2024

View reviewed changes

avdg81 reviewed Nov 1, 2024

View reviewed changes

rfaasse added 7 commits November 1, 2024 16:11

Processing review comments in the element class itself

a6f2db3

Merge remote-tracking branch 'origin/master' into geo/add-extendible-…

69c088e

…calculators-to-pw-line-element

Processing review comments in the unit tests

a82a1f0

Did some renaming for the calculator class and its function names

e5bd016

Processed review comments for compressibility calculator

8eab040

Processed review comments for permeability calculator

670b44a

Processed review comments for permeability calculator

033d8c8

rfaasse commented Nov 1, 2024

View reviewed changes

rfaasse commented Nov 4, 2024

View reviewed changes

applications/GeoMechanicsApplication/custom_elements/compressibility_calculator.cpp Outdated Show resolved Hide resolved

WPK4FEM previously approved these changes Nov 4, 2024

View reviewed changes

avdg81 reviewed Nov 5, 2024

View reviewed changes

Made if-statements a switch, reverted an unwanted rename and changed …

4b51ff3

…the order of a multiplication

rfaasse dismissed WPK4FEM’s stale review via 4b51ff3 November 5, 2024 10:31

Minor last review comment

606111f

WPK4FEM approved these changes Nov 5, 2024

View reviewed changes

rfaasse requested review from avdg81 and markelov208 November 5, 2024 14:29

markelov208 approved these changes Nov 5, 2024

View reviewed changes

rfaasse merged commit 154939a into master Nov 6, 2024
11 checks passed

rfaasse deleted the geo/add-extendible-calculators-to-pw-line-element branch November 6, 2024 08:04

rfaasse mentioned this pull request Dec 2, 2024

[ARCHIVED] Experiment lhs rhs calculators #12763

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GeoMechanicsApplication] Implement Calculator-based workflow for Pw line element #12778

[GeoMechanicsApplication] Implement Calculator-based workflow for Pw line element #12778

rfaasse commented Oct 24, 2024 •

edited

Loading

WPK4FEM left a comment

WPK4FEM Oct 25, 2024

rfaasse Oct 29, 2024

WPK4FEM Oct 25, 2024

rfaasse Nov 1, 2024

WPK4FEM Oct 28, 2024

rfaasse Nov 1, 2024

WPK4FEM Oct 28, 2024

rfaasse Nov 1, 2024

WPK4FEM Oct 28, 2024

rfaasse Nov 1, 2024

markelov208 left a comment

avdg81 left a comment

rfaasse left a comment

rfaasse Nov 1, 2024

rfaasse Nov 1, 2024

WPK4FEM left a comment

WPK4FEM Nov 4, 2024

avdg81 left a comment

WPK4FEM left a comment

markelov208 left a comment

[GeoMechanicsApplication] Implement Calculator-based workflow for Pw line element #12778

[GeoMechanicsApplication] Implement Calculator-based workflow for Pw line element #12778

Conversation

rfaasse commented Oct 24, 2024 • edited Loading

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

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

avdg81 left a comment

Choose a reason for hiding this comment

rfaasse 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

Choose a reason for hiding this comment

avdg81 left a comment

Choose a reason for hiding this comment

WPK4FEM left a comment

Choose a reason for hiding this comment

markelov208 left a comment

Choose a reason for hiding this comment

rfaasse commented Oct 24, 2024 •

edited

Loading