Skip to content
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] added process to calculate incremental displacement #12474

Merged
merged 6 commits into from
Jul 12, 2024

Conversation

aronnoordam
Copy link
Member

moved calculation of incremental displacement from python to cpp as this was a bottleneck

@aronnoordam aronnoordam changed the title addded process to calculate incremental displacement [GeoMechanicsApplication] added process to calculate incremental displacement Jun 24, 2024
@aronnoordam aronnoordam added C++ Performance GeoMechanics Issues related to the GeoMechanicsApplication labels Jun 24, 2024
KRATOS_TRY

block_for_each(mrModelPart.Nodes(), [&](Node& rNode) {
rNode.FastGetSolutionStepValue(INCREMENTAL_DISPLACEMENT) = rNode.FastGetSolutionStepValue(DISPLACEMENT, 0) - rNode.FastGetSolutionStepValue(DISPLACEMENT, 1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
rNode.FastGetSolutionStepValue(INCREMENTAL_DISPLACEMENT) = rNode.FastGetSolutionStepValue(DISPLACEMENT, 0) - rNode.FastGetSolutionStepValue(DISPLACEMENT, 1);
noalias(rNode.FastGetSolutionStepValue(INCREMENTAL_DISPLACEMENT)) = rNode.FastGetSolutionStepValue(DISPLACEMENT, 0) - rNode.FastGetSolutionStepValue(DISPLACEMENT, 1);

Copy link
Member

@AlejandroCornejo AlejandroCornejo Jun 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will also save some time

@AlejandroCornejo
Copy link
Member

AlejandroCornejo commented Jun 24, 2024

Check also the kratos\utilities\variable_utils.cpp since it also includes many optimized auxiliar implementations of this kind

@aronnoordam
Copy link
Member Author

Check also the kratos\utilities\variable_utils.cpp since it also includes many optimized auxiliar implementations of this kind

will do, thanks

Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A nice and straightforward change. Well done! In addition to my minor suggestions, may I ask you to add a C++ unit test for this new process, since no tests have been added to cover it? Thank you!

…lculate_incr_disp_process

# Conflicts:
#	applications/GeoMechanicsApplication/custom_python/add_custom_processes_to_python.cpp
@aronnoordam aronnoordam enabled auto-merge July 10, 2024 09:29
Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only have a few very minor comments regarding the unit test. Apart from that, I believe this PR is ready to go. Thanks for the work done @aronnoordam !

Co-authored-by: Anne van de Graaf <[email protected]>
Copy link
Contributor

@avdg81 avdg81 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR looks ready to go.

@aronnoordam aronnoordam merged commit a37e424 into master Jul 12, 2024
9 of 11 checks passed
@aronnoordam aronnoordam deleted the geo/calculate_incr_disp_process branch July 12, 2024 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ GeoMechanics Issues related to the GeoMechanicsApplication Performance
Projects
None yet
3 participants