[GeoMechanicsApplication] Add micro climate condition #11890

rfaasse · 2023-12-14T08:57:00Z

📝 Description
This PR adds a new condition for microclimate, including effects like solar radiation, air temperature and wind speed. The new class is based on the work done by @johnvanesch @mnabideltares. This PR also adds test cases (both on an integration and unit level).

In order to compile, I had to make the following changes: - `T_condition.hpp` was renamed to `T_condition.h`. - Base class `TCondition` was renamed to `GeoTCondition`.

In the base class, `CalculateLocalSystem` is `public` as well, rather than `protected`. By making it `public`, it becomes easier to unit test the class.

In the base class, these member functions are `public` rather than `protected`. By making them `public`, it becomes easier to unit test the class. Added a first unit test that verifies no exception is thrown when a micro-climate condition is initialized.

The test checks whether calling `InitializeSolutionStep`, given a condition with dummy properties, does not throw an exception. The unit test for `Initialize` now also uses a 2-noded condition.

Copied the code from Mohamed's branch and made the following changes: - Renamed the conditions by prepending "Geo" to the original names. - Renamed the data members by adding "Geo" to their names.

Previously, in the test for `InitializeSolutionStep`, the micro-climate condition was created directly. It is, however, more convenient to let the model part take care of that.

By extracting several helper functions, the test body has become a lot more compact. Using this setup will make it easier to create similar tests for different condition configurations.

Test whether no errors occur when initializing the solution step.

Previously, all nodes were created on a line. This works for line conditions, but not for triangular or quadrilateral ones. With these changes we can now pass a function object that creates the nodes for us. The unit tests have been adapted such that they now use this mechanism.

To make the function that creates a micro-climate condition for testing more generally applicable, it now has a parameter for passing the dimension size. Also added a unit test to verify that when initializing the solution step of a micro-climate condition acting on a linear triangle executes without raising any errors.

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a quadratic triangle executes without raising any errors. The helper function that creates the nodes for a triangle has been extended such that it can also create the mid-side nodes of a quadratic triangle.

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a linear quadrilateral executes without raising any errors.

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a serendipity element executes without raising any errors.

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a LaGrange element executes without raising any errors.

The unit tests verify the left hand side matrix and the right hand side vector calculated for a line micro-climate condition with two nodes. To carry out the calculation, we also had to add solution step variables AIR_HUMIDITY, PRECIPITATION, and DT_TEMPERATURE.

Use a quadratic line condition rather than a linear one. The idea to is have a test for each shape of condition (i.e. line, triangle, and quadrilateral). At the same time, we would like to limit the tests to quadratic shape functions only (to avoid an explosion of tests).

Only quadratic shape functions are tested. To avoid repetition of test code, we now test both the LHS matrix and the RHS vector that have been calculated in the same unit test. As a result, two helper functions were no longer needed and they have been removed. Since we also need to compare a few computed zeros, we occasionally use an absolute tolerance rather than a relative one.

To avoid having a mix of `public` and `private` data members, as was found by SonarQube.

The `override` did exactly the same thing as the base class implementation.

Also removed two lines of commented out code.

- Removed some redundant blank lines. - Ensure there is a space between `//` and the actual comment. - Removed a redundant pair of parenthesis. - Removed some commented out code.

- Return the result using the return value rather than using an out argument. - Pass built-in types by value rather than by reference to const.

WPK4FEM

Impressive testing!
The restructuring probably made clear that some more improvements can be made.
This implementation relies on input being in a precisely defined set of units that hardly can be checked with input verification. The redefinition of already existing material parameters is dangerous.

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

WPK4FEM · 2023-12-14T12:22:45Z

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

+}
+
+template <unsigned int TDim, unsigned int TNumNodes>
+void TMicroClimateFluxCondition<TDim, TNumNodes>::CalculateAndAddRHS(


From the coding I read that these are the integration point contributions for a flux based and a temperature based part. The flux and the temperature are given as element DOF vectors, which I do not think change per integration point. Therefore I wonder if setting up elementmatrices fully and then doing the matrix vector multiplications once i.s.o. number of integration point times is more efficient.

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

applications/GeoMechanicsApplication/python_scripts/geomechanics_solver.py

WPK4FEM · 2023-12-14T13:18:34Z

...MechanicsApplication/tests/test_thermal_element/test_micro_climate_1/MaterialParameters.json

+        "THERMAL_CONDUCTIVITY_WATER"       : 0.6,
+        "LONGITUDINAL_DISPERSIVITY"        : 0.0,
+        "TRANSVERSE_DISPERSIVITY"          : 0.0,
+        "SOLID_COMPRESSIBILITY"            : 1.0,
+        "A1_COEFFICIENT"                   : 0.000000e+00,
+        "A2_COEFFICIENT"                   : 0.000000e+00,
+        "A3_COEFFICIENT"                   : 0.000000e+00,
+        "ALPHA_COEFFICIENT"                : 1.900000e-01,
+        "QF_COEFFICIENT"                   : 0.000000e+00,
+        "SMIN_COEFFICIENT"                 : 0.000000e+00,
+        "SMAX_COEFFICIENT"                 : 4.800000e-04
+      },


These are new and should now appear on our wiki.

I added to the documentation issue, we should add this to the wiki

WPK4FEM · 2023-12-14T13:19:14Z

...oMechanicsApplication/tests/test_thermal_element/test_micro_climate_1/ProjectParameters.json

+      }
+    }]
+  },
+  "processes": {


the processes for new variables deserve a place on our wiki.

I added to the documentation issue, we should add this to the wiki

WPK4FEM · 2023-12-14T13:20:53Z

...oMechanicsApplication/tests/test_thermal_element/test_micro_climate_2/ProjectParameters.json

+            "skin_output"                : false,
+            "plane_output"               : [],
+            "nodal_results"              : ["TEMPERATURE"],
+            "gauss_point_results"        : []


I have not seen HEAT_FLUX output yet. It would be a natural thing to have, like we also have fluid flux.

I created a draft issue on our product backlog to do this

WPK4FEM · 2023-12-14T13:21:49Z

...oMechanicsApplication/tests/test_thermal_element/test_micro_climate_2/ProjectParameters.json

+      "process_name"                     : "ApplyScalarConstraintTableProcess",
+      "Parameters" : {
+        "model_part_name"                : "PorousDomain.TopThermo",
+        "variable_name"                  : "AIR_TEMPERATURE",


can we output and display these new variables? that would be a natural addition for model checking.

I'm not sure, I created a draft issue on our product backlog to check/implement this

markelov208

Richard, very well done. Just a couple of comments.

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

applications/GeoMechanicsApplication/custom_python/geo_mechanics_python_application.cpp

applications/GeoMechanicsApplication/tests/cpp_tests/test_T_micro_climate_flux_condition.cpp

It is now part of the condition utilities. As a result, the newly introduced function `MakeVector` had become redundant, which has been removed now.

As a result, we no longer need the newly introduced function `MakeDiagonalMatrix`.

Eliminated a "raw" loop by selecting a more appropriate constructor.

rfaasse

Only a few minor comments left when going through the code a last time.

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

applications/GeoMechanicsApplication/custom_conditions/micro_climate_constants.h

applications/GeoMechanicsApplication/geo_mechanics_application.h

applications/GeoMechanicsApplication/python_scripts/geomechanics_solver.py

Parameters `LONGITUDINAL_DISPERSIVITY` and `TRANSVERSE_DISPERSIVITY` are not being used by the geomechanical code at all. Therefore, those have been removed from the new input files for integration testing.

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.h

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

For some reason, in the previous implementation, each node was implicitly converted to the number 1 (WTF?). Now we use the `std::iota` algorithm to generate a sequence of local node indices and feed that to the `std::accumulate` algorithm.

It used to call the member function of an indirect base class. Now it calls the one from its direct base class.

…-climate-condition

Don't forget to specify the template arguments.

WPK4FEM

I think it is good to do the remaining things after consultation with Mohamed. Lets go ahead merging the current status.

WPK4FEM · 2023-12-15T14:39:03Z

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

+    // Eq 5.31
+    const auto subsurface_heat_flux =
+        NetRadiation - sensible_heat_flux_right - latent_heat_flux +
+        mBuildEnvironmentRadiation - SurfaceHeatStorage;


misspelled? built environment

WPK4FEM · 2023-12-15T14:45:49Z

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

+        r_geom[NodeIndex].FastGetSolutionStepValue(AIR_TEMPERATURE);
+    const auto absorbed_long_wave_radiation =
+        EffectiveEmissivity * BoltzmannCoefficient *
+        std::pow(atmospheric_temperature + 273.15, 4.0);


The 273.15 shift from Celsius to Kelvin may deserve to be a named constant.

markelov208

Anne and Richard, great work. Just one comment on the variable name.

markelov208 · 2023-12-15T14:29:04Z

applications/GeoMechanicsApplication/custom_conditions/T_microclimate_flux_condition.cpp

+            array_1d<double, TNumNodes>{row(r_N_container, integration_point_index)};
+
+        // Compute weighting coefficient for integration
+        const auto IntegrationCoefficient =


should it be integration_coefficient?

avdg81 added 30 commits December 7, 2023 13:27

Added Mohamed's micro-climate condition "as-is"

908c0bb

In order to compile, I had to make the following changes: - `T_condition.hpp` was renamed to `T_condition.h`. - Base class `TCondition` was renamed to `GeoTCondition`.

Made CalculateLocalSystem a public member

04ba84a

In the base class, `CalculateLocalSystem` is `public` as well, rather than `protected`. By making it `public`, it becomes easier to unit test the class.

Added a first unit test for InitializeSolutionStep

40a701a

The test checks whether calling `InitializeSolutionStep`, given a condition with dummy properties, does not throw an exception. The unit test for `Initialize` now also uses a 2-noded condition.

Registered micro-climate condition in the GeoMechanicsApplication

d26e80a

Copied the code from Mohamed's branch and made the following changes: - Renamed the conditions by prepending "Geo" to the original names. - Renamed the data members by adding "Geo" to their names.

Let the model part create the micro-climate condition

e4858bf

Previously, in the test for `InitializeSolutionStep`, the micro-climate condition was created directly. It is, however, more convenient to let the model part take care of that.

Refactored the unit test for InitializeSolutionStep

87b76fe

By extracting several helper functions, the test body has become a lot more compact. Using this setup will make it easier to create similar tests for different condition configurations.

Added a unit test for 3-noded (line) micro-climate conditions

23ecf62

Test whether no errors occur when initializing the solution step.

Added a unit test for 4-noded (line) micro-climate conditions

cc91081

Test whether no errors occur when initializing the solution step.

Added a unit test for 5-noded (line) micro-climate conditions

06e3c08

Test whether no errors occur when initializing the solution step.

Added a test for a condition acting on a linear quadrilateral

b3f0f47

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a linear quadrilateral executes without raising any errors.

Added a test for a condition acting on a serendipity element

f15440c

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a serendipity element executes without raising any errors.

Added a test for a condition acting on a LaGrange element

ec35856

The unit test verifies that when initializing the solution step of a micro-climate condition acting on a LaGrange element executes without raising any errors.

Added a test to verify the local system of a quadrilateral condition

e95a0fd

Renamed a data member to comply with the Kratos Style Guide

56e0f77

Made a public data member private

124d6de

To avoid having a mix of `public` and `private` data members, as was found by SonarQube.

Removed an unused data member

db184c5

Removed a needless override

ee9732b

The `override` did exactly the same thing as the base class implementation.

Removed a data member that was only assigned, but never used

779944f

Also removed two lines of commented out code.

Inlined member function CalculateKinematics

cafd671

Removed some redundant comments, separation lines, and blank lines

841c4e2

Improved several minor issues

4b44a2a

- Removed some redundant blank lines. - Ensure there is a space between `//` and the actual comment. - Removed a redundant pair of parenthesis. - Removed some commented out code.

Improved the signature of a member function

bcf309d

- Return the result using the return value rather than using an out argument. - Pass built-in types by value rather than by reference to const.

Removed trailing white spaces (codacy)

b67b4ea

WPK4FEM reviewed Dec 14, 2023

View reviewed changes

markelov208 reviewed Dec 14, 2023

View reviewed changes

avdg81 and others added 5 commits December 14, 2023 14:56

Moved the calculation of the integration coefficient

3d08ab0

It is now part of the condition utilities. As a result, the newly introduced function `MakeVector` had become redundant, which has been removed now.

Reuse the function to calculate the integration coefficient

a884b08

Use elemen_prod for elementwise vector multiplications

9cb81db

As a result, we no longer need the newly introduced function `MakeDiagonalMatrix`.

Simplified a function's implementation

bbcc26a

Eliminated a "raw" loop by selecting a more appropriate constructor.

Rewrote summing the net radiation using std::accumulate

45b13d1

rfaasse assigned avdg81 and rfaasse Dec 15, 2023

rfaasse commented Dec 15, 2023

View reviewed changes

avdg81 and others added 4 commits December 15, 2023 10:49

Removed two unused material parameters from the new input files

8e3164d

Parameters `LONGITUDINAL_DISPERSIVITY` and `TRANSVERSE_DISPERSIVITY` are not being used by the geomechanical code at all. Therefore, those have been removed from the new input files for integration testing.

Processing part of the review comments

b2c88ea

Implemented save/load for the microclimate condition members

a8efb38

Reading water density from file instead of setting it a constant

ad69262

avdg81 reviewed Dec 15, 2023

View reviewed changes

avdg81 added 6 commits December 15, 2023 14:07

Renamed a local variable

4739d67

Prepended the micro climate condition class name with Geo

f8a9bfd

Corrected the qualification of the Initialize call

d0df38b

It used to call the member function of an indirect base class. Now it calls the one from its direct base class.

Replaced a <Tab> by spaces

2f46c12

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

50cb9ae

…-climate-condition

avdg81 requested review from WPK4FEM and markelov208 December 15, 2023 13:56

avdg81 enabled auto-merge (squash) December 15, 2023 13:58

Fixed a compile-time error from GCC and Clang

bdd4195

Don't forget to specify the template arguments.

WPK4FEM approved these changes Dec 15, 2023

View reviewed changes

markelov208 reviewed Dec 15, 2023

View reviewed changes

avdg81 merged commit 327d0a7 into master Dec 15, 2023
16 of 17 checks passed

avdg81 deleted the geo/11866-add-micro-climate-condition branch December 15, 2023 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GeoMechanicsApplication] Add micro climate condition #11890

[GeoMechanicsApplication] Add micro climate condition #11890

rfaasse commented Dec 14, 2023

WPK4FEM left a comment

WPK4FEM Dec 14, 2023

WPK4FEM Dec 14, 2023

rfaasse Dec 15, 2023

WPK4FEM Dec 14, 2023

rfaasse Dec 15, 2023

WPK4FEM Dec 14, 2023

rfaasse Dec 15, 2023

WPK4FEM Dec 14, 2023

rfaasse Dec 15, 2023

markelov208 left a comment

rfaasse left a comment

WPK4FEM left a comment

WPK4FEM Dec 15, 2023

WPK4FEM Dec 15, 2023

markelov208 left a comment

markelov208 Dec 15, 2023

[GeoMechanicsApplication] Add micro climate condition #11890

[GeoMechanicsApplication] Add micro climate condition #11890

Conversation

rfaasse commented Dec 14, 2023

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

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

markelov208 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment