-
Notifications
You must be signed in to change notification settings - Fork 8
test enzyme derivatives with hyperelastic neohookean potential #148
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: develop
Are you sure you want to change the base?
Conversation
ebchin
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.
Nicely done @rjl33! Just a few things to clean up and it should be ready to merge.
|
|
||
| class EnzymeNeohookeanTest : public testing::Test { | ||
| protected: | ||
| double F_all[27] = { 1.0, 0.5, 0.0, 0.0, 1.2, 0.1, 0.0, 0.0, 1.0, 1.1, 0.7, 0.1, 0.2, 1.4, |
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.
Try using //clang-format off and //clang-format on to keep the size 3x9. See this as an example.
| #include "axom/slic/interface/slic.hpp" | ||
| #include <cmath> | ||
| #include <limits> | ||
| #include "tribol/common/Enzyme.hpp" |
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.
It looks like a lot of these #includes aren't needed anymore. Try taking out as many as you can
| run_fwd_FD( E, mu, lambda, d2W_d2E, N, h ); | ||
| run_hand_fwd( E, mu, lambda, d2W_d2E, N ); | ||
|
|
||
| return 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.
Very clear and nicely written. Can you add some timing results to the PR description?
src/tests/enzyme_neohookean.cpp
Outdated
| double dW2_fwd_fwd[81] = { 0.0 }; | ||
| second_deriv_fwd_FD( E_all + i * 9, mu, lambda, dW2_FD ); | ||
| second_deriv_fwd_fwd( E_all + i * 9, mu, lambda, dW2_fwd_fwd ); | ||
| for ( int j = 0; j < 9; ++j ) { |
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.
Should the for loop go to 81? Let's chat about writing a template function or using std::function to reduce this code duplication
| double dW2_hand_fwd[81] = { 0.0 }; | ||
| second_deriv_fwd_FD( E_all + i * 9, mu, lambda, dW2_FD ); | ||
| second_deriv_hand_fwd( E_all + i * 9, mu, lambda, dW2_hand_fwd ); | ||
| for ( int j = 0; j < 9; ++j ) { |
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.
Can you add a comment why we are only looking at entries 0-8?
| } | ||
| } | ||
|
|
||
| void hand_code_deriv( double* E, double mu, double lambda, double* S ) |
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.
Maybe stress_handcoded?
src/tests/neohookean_common.hpp
Outdated
| } | ||
|
|
||
|
|
||
| void run_fwd_mode(double* E, double mu, double lambda, double* dw_df, int N) { |
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.
These functions (L298-403) are example-specific, so it might make sense to move them to neohookean_timing.cpp
src/tests/neohookean_common.hpp
Outdated
| std::cout << axom::fmt::format( "Time to calc hand_diff: {0:f}ms", timer.elapsedTimeInMilliSec() ) << std::endl; | ||
| } | ||
|
|
||
| void run_fwd_fwd(double* E, double mu, double lambda, double* S, int N) { |
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.
Change S to something like dSdE
src/tests/neohookean_common.hpp
Outdated
| std::cout << axom::fmt::format( "Time to calc fwd_fwd_diff: {0:f}ms", timer.elapsedTimeInMilliSec() ) << std::endl; | ||
| } | ||
|
|
||
| void run_rev_fwd(double* E, double mu, double lambda, double* S, int N) { |
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.
Change this S too, same with all the other second deriv functions.
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 chat about reducing some of this code duplication with either a template or a std::function parameter.
| common_plane_gpu.cpp | ||
| common_plane.cpp | ||
| mortar_lm_patch_test.cpp | ||
| neohookean_timing.cpp |
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 will need to turn this test off when Tribol is built without Enzyme. Try removing this from the list and adding this after: blt_list_append(TO contact_examples ELEMENTS neohookean_timing.cpp IF TRIBOL_USE_ENZYME )
Speed test for enzyme derivatives and finite difference checks for correctness