-
Notifications
You must be signed in to change notification settings - Fork 248
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
[GeoMechanicsApplication] Incremental displacement variable, output and integration test. #12288
Conversation
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.
Very nice additional variable, clearly (unit)tested and added to all workflows! I have a few suggestions, but this seems like a nice complete feature to me!
applications/GeoMechanicsApplication/custom_workflows/time_loop_executor.hpp
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/geomechanics_U_Pw_solver.py
Outdated
Show resolved
Hide resolved
applications/GeoMechanicsApplication/python_scripts/geomechanics_analysis.py
Show resolved
Hide resolved
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.
This will help to get a better understanding how a failure mechanism develops. The code you've added is clear and compact. Thank you for extending the Python output as well as the C++ output.
""" | ||
Calculates incremental displacement | ||
:param node: | ||
:return: | ||
""" |
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.
Feel free to remove these lines, since they have no added value in my opinion. The method's name tells it all.
incremental_displacement = node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 0) - \ | ||
node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 1) |
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.
In general, I would try to avoid using the line continuation marker. You can do that as follows:
incremental_displacement = node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 0) - \ | |
node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 1) | |
incremental_displacement = (node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 0) - | |
node.GetSolutionStepValue(KratosMultiphysics.DISPLACEMENT, 1)) |
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.
Done
p_node->GetSolutionStepValue(DISPLACEMENT,1) = displacement_start_time_step; | ||
p_node->GetSolutionStepValue(DISPLACEMENT,0) = displacement_end_time_step; |
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.
Nitpicking:
p_node->GetSolutionStepValue(DISPLACEMENT,1) = displacement_start_time_step; | |
p_node->GetSolutionStepValue(DISPLACEMENT,0) = displacement_end_time_step; | |
p_node->GetSolutionStepValue(DISPLACEMENT, 1) = displacement_start_time_step; | |
p_node->GetSolutionStepValue(DISPLACEMENT, 0) = displacement_end_time_step; |
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.
Done.
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.
Done.
KRATOS_TEST_CASE_IN_SUITE(ComputeIncrementalDisplacementField, KratosGeoMechanicsFastSuite) | ||
{ | ||
Model model; | ||
auto& model_part = CreateDummyModelPart(model); |
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.
Strictly speaking, we should prefix this name with r_
to comply with the Kratos Style Guide. Having said that, I can imagine that this minor inconsistency appears in other tests in the same file as well. So we could either correct those as well, or do that in a separate PR. I'll leave that up to you.
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.
Done for this .cpp file.
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.
Looks good to me! Thanks for incorporating the review comments and for adding this nice bit of functionality!
…nd integration test. (#12288) * Incremental displacement variable, output and integration test. * Output enabled fhrough the C++ route too.
📝 Description
For clear display of failure mechanisms we would like to inspect the extra deformations computed during one step, additional to the displacements since the start of a stage.
🆕 Changelog