[GeoMechanicsApplication] Fix prescribed acceleration #12208

rfaasse · 2024-03-19T10:27:36Z

📝 Description
This PR aims to fix the issue with prescribed accelerations which were overwritten by the time integration schemes. Functionality is added to the schemes to check if a variable is fixed, and in that case skip the update. Unit tests and 3 integration tests are added to ensure this works correctly for all prescribed derivatives (i.e. first and second derivative of the vector variables and the first derivative of the scalar variable). A README.md is added to the new folder of tests to document its assertions.

…ns being overwritten by the time integration scheme

… fixed

…nal setting of the variables.

…e value is not changed after scheme update

…g the value is not changed after scheme update

rfaasse · 2024-03-19T13:19:01Z

applications/GeoMechanicsApplication/custom_strategies/schemes/backward_euler_scheme.hpp

@@ -52,13 +53,13 @@ class BackwardEulerScheme : public GeoMechanicsTimeIntegrationScheme<TSparseSpac
        KRATOS_TRY

        block_for_each(rModelPart.Nodes(), [this](Node& rNode) {
-            // For the Backward Euler schemes the first derivatives should be


A similar comment is already placed in line 76, so I thought it was redundant to leave it here

...pplication/tests/prescribed_derivative_tests/prescribed_acceleration/MaterialParameters.json

rfaasse · 2024-03-19T14:01:46Z

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

+        # Code here will be placed AFTER every test in this TestCase.
+        pass
+
+    def test_prescribed_acceleration(self):


If there are any other things to test besides the prescribed variables themselves, please let me know!

…ribed-acceleration

avdg81

Very nice and clear improvement backed up by excellent tests. Thanks for the great work Richard!

applications/GeoMechanicsApplication/custom_utilities/node_utilities.cpp

applications/GeoMechanicsApplication/custom_utilities/node_utilities.h

applications/GeoMechanicsApplication/custom_utilities/variables_utilities.cpp

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

avdg81 · 2024-03-20T08:32:09Z

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

+    def test_prescribed_acceleration(self):
+        """
+        This test checks if the prescribed acceleration in the x-direction is correctly applied to the model
+        :return:


I'm not sure what this line means. Do we need it?

Removed the :return:, was there from copied test

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

WPK4FEM

Dear Richard,
This seems quite consistent to me ( even if we know that the integration tests still suffer from the not yet understood ill condition ).
I appreciate the extensive unit testing.
Wijtze Pieter

WPK4FEM · 2024-03-19T16:03:18Z

applications/GeoMechanicsApplication/custom_strategies/schemes/backward_euler_scheme.hpp

+            const auto updated_first_time_derivative =
+                CalculateDerivative(r_second_order_vector_variable.instance, rNode);
+
+            NodeUtilities::ApplyUpdatedVectorVariableToNonFixedComponents(


I'm wondering:
Looks more like VectorDOFComponentUtilities then NodeUtilities to me,
However, we are passing rNode to get to the fixed D.O.F.

Indeed, due to that last point, I'd propose to keep it like it is

...pplication/tests/prescribed_derivative_tests/prescribed_acceleration/MaterialParameters.json

WPK4FEM · 2024-03-19T16:14:00Z

...Application/tests/prescribed_derivative_tests/prescribed_acceleration/ProjectParameters.json

+          "active": [
+            false,
+            true,
+            false
+          ],
+          "is_fixed": [
+            false,
+            true,
+            false
+          ],
+          "value": [
+            0.0,
+            0.0,
+            0.0
+          ],
+          "table": [
+            0,
+            0,
+            0
+          ]
+        }


very long way of formatting.
"active": [false, true, false],
"is_fixed": [false, true, false],
"value": [0.0, 0.0, 0.0],
"table": [0, 0, 0,],
is a log clearer to me.

This pattern repeats in several places.

WPK4FEM · 2024-03-20T09:43:52Z

applications/GeoMechanicsApplication/tests/prescribed_derivative_tests/MeshStructure.svg

@@ -0,0 +1,3197 @@
+<?xml version="1.0" encoding="UTF-8" standalone="no"?>


Peculiar that the internal mid nodes are missing. However. for the boundary conditions, it may be clearer to omit the nodes and the internal mesh lines.

rfaasse

Processed the review comments of @avdg81, fixed function for Release build and formatted ProjectParameters.json to make them more compact.

What's left is fixing the integration tests.

applications/GeoMechanicsApplication/custom_utilities/variables_utilities.cpp

applications/GeoMechanicsApplication/custom_utilities/node_utilities.cpp

applications/GeoMechanicsApplication/custom_utilities/node_utilities.h

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

rfaasse · 2024-03-20T10:37:47Z

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

+    def test_prescribed_acceleration(self):
+        """
+        This test checks if the prescribed acceleration in the x-direction is correctly applied to the model
+        :return:


Removed the :return:, was there from copied test

applications/GeoMechanicsApplication/tests/test_prescribed_derivatives.py

WPK4FEM

Seems consistent to me. my remarks are mainly on the test description.

WPK4FEM · 2024-03-22T10:49:40Z

applications/GeoMechanicsApplication/tests/prescribed_derivative_tests/README.md

+      modulus
+      of 150000 kPa and a Poisson ratio of 0.3.
+
+No conditions or loads are applied.


Not true, the bottom has a Dirichlet condition for a, v or dp/dt. That is a form of loading.

Good point, added

WPK4FEM · 2024-03-22T10:50:23Z

applications/GeoMechanicsApplication/tests/prescribed_derivative_tests/README.md

+| 0.2  | 0.01   |
+
+- Constraints:
+    - The Y displacements at the sides and bottom are fixed.


In this way a column that can only react as shear column is created.

Thanks for the addition, added!

…ble to other two tests

…del author

WPK4FEM

Complete.

avdg81

This is a nice and clear improvement, backed up by sound tests. Thanks for all the hard work that you have done.

avdg81 · 2024-03-22T13:53:47Z

applications/GeoMechanicsApplication/tests/cpp_tests/test_newmark_dynamic_U_Pw_scheme.cpp

@@ -229,4 +229,69 @@ KRATOS_TEST_CASE_IN_SUITE(NewmarkDynamicUPwSchemePredict_UpdatesVariablesDerivat
        expected_dt_water_pressure);
 }

+KRATOS_TEST_CASE_IN_SUITE(NewmarkDynamicUPwSchemePredict_DoesNotUpdateFixedSecondDerivativeVectorVariable,


Shouldn't this name refer to Update rather than Predict? The same question applies to the next two unit tests in this file.

You're right, I started with Predicts and changed to Updates later, I'll check all added tests

…and clang-formatted the test file

avdg81

I believe we're ready to go.

rfaasse added 18 commits March 15, 2024 16:04

Added unit test and crude fix for the issue of prescribed acceleratio…

150f21c

…ns being overwritten by the time integration scheme

Generalized the solution to all second order vector variables

29c792b

Added some comments

71c6ec1

Fixed updating the scalar derivatives only when the derivative is not…

eddd341

… fixed

Fixed newmark prescribed velocity

19c50ac

Refactored only applying update to non-fixed components

c0b70d1

Fix code smell

8f6eaee

Small refactoring in the backward euler schemes to allow for conditio…

5a13875

…nal setting of the variables.

Added backward euler test for fixed dt water pressure and checking th…

ae6a145

…e value is not changed after scheme update

Added backward euler test for fixed acceleration pressure and checkin…

137545b

…g the value is not changed after scheme update

Fix typo

c965742

Fix typo

ab1a5da

Added unit tests for node utilities

9d7334f

Added integration test for prescribed acceleration

1477a66

Added integration test for prescribed velocity

82404d5

Added integration test for prescribed dt_water_pressure

0ccd576

Added descriptions to tests in files

59fe262

Added README.md to describe the tests

dac53fd

rfaasse commented Mar 19, 2024

View reviewed changes

...pplication/tests/prescribed_derivative_tests/prescribed_acceleration/MaterialParameters.json Show resolved Hide resolved

rfaasse commented Mar 19, 2024

View reviewed changes

rfaasse added 2 commits March 19, 2024 15:03

Minor fixes and added tests for the added variables utils

5d601a9

Merge remote-tracking branch 'origin/master' into geo/12184-fix-presc…

c0551d9

…ribed-acceleration

rfaasse requested review from avdg81 and WPK4FEM March 19, 2024 14:05

rfaasse marked this pull request as ready for review March 19, 2024 14:08

rfaasse linked an issue Mar 19, 2024 that may be closed by this pull request

[GeoMechanicsApplication] Prescribed acceleration does not result in the expected acceleration values in the respective nodes. #12184

Closed

avdg81 reviewed Mar 20, 2024

View reviewed changes

WPK4FEM reviewed Mar 20, 2024

View reviewed changes

Processing review comments

06a3807

rfaasse added 2 commits March 20, 2024 11:53

Reformat json files to make them more compact

3875907

Added 'Has' check for GetComponentFromVectorVariable

e0720cf

rfaasse commented Mar 20, 2024

View reviewed changes

rfaasse added 8 commits March 20, 2024 12:10

Used structured binding to compact code

777ce69

Processed review commetns

d22ef12

removed redundant comment

464b453

Attempt to fix pipeline build issue

b1d5e66

Removed structural binding, as this does not work with boost::combine

327de65

Fixed prescribed acceleration test

6773cda

Fixed prescribed velocity test

7b92516

Improved image based on review comments

d68d8dd

rfaasse requested review from avdg81 and WPK4FEM March 22, 2024 09:58

WPK4FEM reviewed Mar 22, 2024

View reviewed changes

rfaasse added 2 commits March 22, 2024 14:47

Made changes to the prescribed_dt_water_pressure test to make compara…

448403c

…ble to other two tests

Processed review comments for the README.md, including credits for mo…

abebd09

…del author

WPK4FEM previously approved these changes Mar 22, 2024

View reviewed changes

avdg81 reviewed Mar 22, 2024

View reviewed changes

Changed name of test to reflect tested functionality more accurately …

e124c0f

…and clang-formatted the test file

rfaasse dismissed WPK4FEM’s stale review via e124c0f March 22, 2024 15:08

rfaasse enabled auto-merge (squash) March 22, 2024 15:13

rfaasse requested review from avdg81 and WPK4FEM March 22, 2024 16:18

avdg81 approved these changes Mar 22, 2024

View reviewed changes

rfaasse merged commit 1166a00 into master Mar 22, 2024
11 checks passed

rfaasse deleted the geo/12184-fix-prescribed-acceleration branch March 22, 2024 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GeoMechanicsApplication] Fix prescribed acceleration #12208

[GeoMechanicsApplication] Fix prescribed acceleration #12208

rfaasse commented Mar 19, 2024 •

edited

Loading

rfaasse Mar 19, 2024

rfaasse Mar 19, 2024

avdg81 left a comment

avdg81 Mar 20, 2024

rfaasse Mar 20, 2024

WPK4FEM left a comment

WPK4FEM Mar 19, 2024

rfaasse Mar 22, 2024

WPK4FEM Mar 19, 2024

rfaasse Mar 20, 2024

WPK4FEM Mar 20, 2024

rfaasse Mar 22, 2024

rfaasse left a comment

rfaasse Mar 20, 2024

WPK4FEM left a comment

WPK4FEM Mar 22, 2024

rfaasse Mar 22, 2024

WPK4FEM Mar 22, 2024

rfaasse Mar 22, 2024

WPK4FEM left a comment

avdg81 left a comment

avdg81 Mar 22, 2024

rfaasse Mar 22, 2024

avdg81 left a comment

		@@ -0,0 +1,3197 @@
		<?xml version="1.0" encoding="UTF-8" standalone="no"?>

[GeoMechanicsApplication] Fix prescribed acceleration #12208

[GeoMechanicsApplication] Fix prescribed acceleration #12208

Conversation

rfaasse commented Mar 19, 2024 • edited Loading

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

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

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

rfaasse left a 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

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

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

avdg81 left a comment

Choose a reason for hiding this comment

rfaasse commented Mar 19, 2024 •

edited

Loading