-
Notifications
You must be signed in to change notification settings - Fork 273
[GeoMechanicsApplication] Extend extrapolator to support lines #14030
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| expected_extrapolation_matrix <<= UblasUtilities::CreateMatrix({{ 0.923275, 0.444444,-0.367719}, | ||
| {-0.367719, 0.444444, 0.923275}}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still check these values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
outdated, now an analytical result.
| // Used the integration method found in the 2d3 Timoshenko Beam element | ||
| constexpr auto integration_method = GeometryData::IntegrationMethod::GI_GAUSS_3; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this integration method is strange for a 2D2 (same for the 2D3 case), but for the moment I just took the same as the timoshenko beam (since that's what we'll use it for)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Outdated, now using GAUSS_2
markelov208
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Richard, it looks the extension was very easy to do. ;)
| Matrix result(rInitializerList.size(), rInitializerList.begin()->size(), 0.0); | ||
| for (std::size_t i = 0; const auto& r_row : rInitializerList) { | ||
| std::ranges::copy(r_row, row(result, i).begin()); | ||
| ++i; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function does not check the input for correctness. Is it on purpose? To have it fast, right?
| namespace Kratos::Testing | ||
| { | ||
|
|
||
| KRATOS_TEST_CASE_IN_SUITE(NodalExtrapolator_GivesCorrectExtrapolationMatrix_For2D3NLine, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no test when numbers of nodes and gauss points are equal. The output is expected. However, is it useful to have such a test to increase the code coverage? Or just in case?
avdg81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Richard,
Thank you very much for extending the linear nodal extrapolator such that it can also handle line geometries. The changes are clear and nicely unit-tested. I only have a few minor suggestions, so I expect we can merge this soon.
| return result; | ||
| } | ||
|
|
||
| Matrix UblasUtilities::CreateMatrix(const std::initializer_list<std::initializer_list<double>>& rInitializerList) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code suggests that we receive a list of rows. Perhaps we can rename the function parameter such that this is clearer? For instance:
| Matrix UblasUtilities::CreateMatrix(const std::initializer_list<std::initializer_list<double>>& rInitializerList) | |
| Matrix UblasUtilities::CreateMatrix(const std::initializer_list<std::initializer_list<double>>& rRows) |
| { | ||
| if (rInitializerList.size() == 0) return {}; | ||
|
|
||
| Matrix result(rInitializerList.size(), rInitializerList.begin()->size(), 0.0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since there are no checks on the sizes of the supplied rows, it may be safer to determine the maximum row length and use that as the number of columns of the matrix. What do you think? Or should we raise an error when the rows don't have one and the same size?
| nodal_extrapolator.CalculateElementExtrapolationMatrix(geometry, integration_method); | ||
|
|
||
| // clang-format off | ||
| Matrix expected_extrapolation_matrix = UblasUtilities::CreateMatrix( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make this one const auto:
| Matrix expected_extrapolation_matrix = UblasUtilities::CreateMatrix( | |
| const auto expected_extrapolation_matrix = UblasUtilities::CreateMatrix( |
| {0.5, 0.5}}); | ||
| // clang-format on | ||
|
|
||
| KRATOS_EXPECT_MATRIX_NEAR(extrapolation_matrix, expected_extrapolation_matrix, 1e-6) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we #include "tests/cpp_tests/test_utilities.h", we can use the default absolute tolerance in the check:
| KRATOS_EXPECT_MATRIX_NEAR(extrapolation_matrix, expected_extrapolation_matrix, 1e-6) | |
| KRATOS_EXPECT_MATRIX_NEAR(extrapolation_matrix, expected_extrapolation_matrix, Defaults::absolute_tolerance) |
📝 Description
Extends the extrapolator to be able to extrapolate lines and adds a regression unit test for it. Next to that, a minor addition is done to the ublas utilities to also be able to generate a matrix.