[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13151

markelov208 · 2025-02-19T20:40:27Z

📝 Description
A brief description of the PR.

extended Check function
extended CheckProperty function and changed the error message
added unit test for Check function

WPK4FEM

Hi Gennady,
Thank you for the extensions, tried to look at it from a user that gets errors point of view. I think there is still a lot of improvement possible on the existing error messages, but that may require a different form of overriding the check functions.
See the remarks.
Regards, Wijtze Pieter

WPK4FEM · 2025-02-20T08:46:02Z

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

+        KRATOS_ERROR_IF(GetProperties()[rVariable] < 0.0 || GetProperties()[rVariable] > MaxValue)
            << rVariable.Name() << " has an invalid value at element " << Id() << std::endl;


It's nice that the element number is given, but the information that leads the user to the right thing to correct is the property Id. Also it would be nice if the error message gives information about which boundary is violated, so information about the boundary value(s) and the value of the property. That required knowledge on whether the upper boundary is a physical one or the default from the code.

Thank you for the user' eye. Property Id is added to the messages. Concerning the max possible value, right now I know only bout porosity. It will be nice to use physical limits then we need a table. Shall we discuss it?

WPK4FEM · 2025-02-20T08:55:06Z

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

@@ -162,7 +164,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) TransientPwLineElement : public Elem
            << "DomainSize smaller than " << min_domain_size << " for element " << Id() << std::endl;
    }


Apparently "DomainSize" means something like element size ( from reading the test with coincident nodes ). Very unexpected naming for, but its in core so just use the GetGeometry().DomainSize(). However the error text can be clearer, i.s.o. of mentioning a DomainSize for an element of which no user has a clue what it is.

I have changed the private function name and the error message. Now they are "CheckElementLength" and "Lenght".

WPK4FEM · 2025-02-20T09:17:46Z

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

+    p_element->GetProperties().SetValue(BULK_MODULUS_SOLID, -1.0E6);
+    KRATOS_EXPECT_EXCEPTION_IS_THROWN(p_element->Check(dummy_process_info),
+                                      "Error: BULK_MODULUS_SOLID has an invalid value at element 4")
+
+    p_element->GetProperties().SetValue(BULK_MODULUS_SOLID, 1.0E6);
+    KRATOS_EXPECT_EXCEPTION_IS_THROWN(
+        p_element->Check(dummy_process_info),
+        "Error: BULK_MODULUS_FLUID does not exist in the material properties at element 4")
+
+    p_element->GetProperties().SetValue(BULK_MODULUS_FLUID, -1.0E6);
+    KRATOS_EXPECT_EXCEPTION_IS_THROWN(p_element->Check(dummy_process_info),
+                                      "Error: BULK_MODULUS_FLUID has an invalid value at element 4")
+
+    p_element->GetProperties().SetValue(BULK_MODULUS_FLUID, 1.0E6);
+    KRATOS_EXPECT_EXCEPTION_IS_THROWN(
+        p_element->Check(dummy_process_info),
+        "Error: DYNAMIC_VISCOSITY does not exist in the material properties at element 4")


There is an interdependency between the bulk moduli of solid, fluid an Biot modulus. The bulk moduli can be used to compute a Biot alfa modulus, that may set some requirements on the relative values of the bulk moduli.

I think it will require an implementation of some kind of a wizard. We can create an issue for it.

rfaasse

Thanks for adding the checks and the extensive test! I have a few comments, but nothing major

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

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

markelov208

Hi Wijtze-Pieter and Richard, many thanks for the reviews. I agree that it is better to have physical grounded limits for the checks. Shall we work on them later?

rfaasse

Looks good to go to me, thanks for processing the review comments!

WPK4FEM

Good for me.

WPK4FEM · 2025-02-21T12:15:05Z

applications/GeoMechanicsApplication/custom_elements/transient_Pw_line_element.h

        KRATOS_ERROR_IF_NOT(GetProperties().Has(rVariable))
-            << rVariable.Name() << " does not exist in the pressure element's properties" << std::endl;
-        KRATOS_ERROR_IF(GetProperties()[rVariable] < 0.0)
-            << rVariable.Name() << " has an invalid value at element " << Id() << std::endl;
+            << rVariable.Name()
+            << " does not exist in the material properties (Id = " << GetProperties().Id()
+            << ") at element " << Id() << std::endl;
+        constexpr auto min_value = 0.0;
+        if (MaxValue.has_value()) {
+            KRATOS_ERROR_IF(GetProperties()[rVariable] < min_value ||
+                            GetProperties()[rVariable] > MaxValue.value())
+                << rVariable.Name() << " of material Id = " << GetProperties().Id() << " at element "
+                << Id() << " has an invalid value " << GetProperties()[rVariable] << " which is outside of the range [ "
+                << min_value << ", " << MaxValue.value() << "]" << std::endl;
+        } else {
+            KRATOS_ERROR_IF(GetProperties()[rVariable] < min_value)
+                << rVariable.Name() << " of material Id = " << GetProperties().Id()
+                << " at element " << Id() << " has an invalid value " << GetProperties()[rVariable]
+                << " which is below the minimum allowed value of " << min_value << std::endl;
+        }
    }


In this way the user is really informed on what is wrong and has a chance of quickly fixing the input. Thank you.

WPK4FEM · 2025-02-21T12:17:00Z

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

+    p_element->GetProperties().SetValue(DENSITY_WATER, -1.0E3);
+    KRATOS_EXPECT_EXCEPTION_IS_THROWN(
+        p_element->Check(dummy_process_info),
+        "DENSITY_WATER of material Id = 0 at element 4 has an invalid value -1000 which is below "


I wonder if material Id = 0 is something that a user created model would have. Non programmers usually start counting at 1. However, all functionality is properly covered here.

Yes, it may happen. If we have a common manual, it can be stressed there.

markelov208 added 3 commits February 19, 2025 10:30

extended Check function

e8cd42f

extended CheckProperty

8787d45

added unit test for Check

0d94684

markelov208 self-assigned this Feb 19, 2025

removed commented lines

56bb627

markelov208 linked an issue Feb 19, 2025 that may be closed by this pull request

[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13000

Closed

changed error text

7257c59

markelov208 requested review from rfaasse, avdg81 and WPK4FEM February 20, 2025 07:40

WPK4FEM reviewed Feb 20, 2025

View reviewed changes

changed DomainSize to ElementLength

cdded7f

rfaasse reviewed Feb 20, 2025

View reviewed changes

markelov208 added 2 commits February 20, 2025 16:11

added Property Id to error messages

273a2d9

replaced hardcoded max value with numerical_limits::max

373b775

markelov208 commented Feb 20, 2025

View reviewed changes

used empty() to remove a code smell

11e1f78

markelov208 requested review from rfaasse and WPK4FEM February 20, 2025 21:46

rfaasse previously approved these changes Feb 21, 2025

View reviewed changes

markelov208 added 2 commits February 21, 2025 10:27

replaced size>0 with empty

dd7efd5

extended error messages

a466732

markelov208 dismissed rfaasse’s stale review via a466732 February 21, 2025 10:26

WPK4FEM approved these changes Feb 21, 2025

View reviewed changes

markelov208 merged commit 412f6c5 into master Feb 21, 2025
11 checks passed

markelov208 deleted the geo/13000-extend-cehck-for-TransientPwLneElement branch February 21, 2025 12:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13151

[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13151

markelov208 commented Feb 19, 2025 •

edited

Loading

WPK4FEM left a comment

WPK4FEM Feb 20, 2025

markelov208 Feb 20, 2025

WPK4FEM Feb 20, 2025

markelov208 Feb 20, 2025

WPK4FEM Feb 20, 2025

markelov208 Feb 20, 2025

rfaasse left a comment

markelov208 left a comment

rfaasse left a comment

WPK4FEM left a comment

WPK4FEM Feb 21, 2025

WPK4FEM Feb 21, 2025

markelov208 Feb 21, 2025

		KRATOS_ERROR_IF(GetProperties()[rVariable] < 0.0 \|\| GetProperties()[rVariable] > MaxValue)
		<< rVariable.Name() << " has an invalid value at element " << Id() << std::endl;

		@@ -162,7 +164,7 @@ class KRATOS_API(GEO_MECHANICS_APPLICATION) TransientPwLineElement : public Elem
		<< "DomainSize smaller than " << min_domain_size << " for element " << Id() << std::endl;
		}

[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13151

[GeoMechanicsApplication] Extend the Check member function of the TransientPwLineElement #13151

Conversation

markelov208 commented Feb 19, 2025 • 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

rfaasse left a comment

Choose a reason for hiding this comment

markelov208 left a comment

Choose a reason for hiding this comment

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

markelov208 commented Feb 19, 2025 •

edited

Loading