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

Reorganize material models with Eigen #318

Merged
merged 35 commits into from
Dec 28, 2024

Conversation

aabrown100-git
Copy link
Collaborator

@aabrown100-git aabrown100-git commented Dec 18, 2024

Current situation

Resolves #297

Release Notes

  • Removes struct implementation using C-arrays (struct_3d_carray() and mat_models_carray.h)
  • Consolidates get_pk2cc() (for struct) and get_pk2cc_dev() (ustruct)
  • Consolidates some common tensor operations in get_pk2cc() into functions
  • Implements get_pk2cc() with fixed-size Eigen matrices and tensors

Documentation

Testing

Code of Conduct & Contributing Guidelines

aabrown100-git and others added 29 commits November 4, 2024 20:51
…update HO-ma reference solutions, remove mat_models_carray.h includes
…to header; using fixed size Matrix and Tensors, instead of just Tensors. Compiles and passes integration tests/unit tests that use NeoHookean
…to S_iso and CC_iso; Add several required tensor operations for Eigen Matrices/Tensors
…auto instead of EigenTensor<nsd> for PP. 2) fix typo for ten_symm_prod_eigen()
…tation, only modifying to use Eigen tensor functions.
@aabrown100-git
Copy link
Collaborator Author

Very nice work, @aabrown100-git!

I saw that there is a mat_models_Dave_array.cpp, which is marked as a new file. Is that just incorrectly tracked by git, or is that really a new file? Do we still want to redundant implementations, Eigen vs. Dave array?

Oops sorry! mat_models_Dave_array.cpp was just a temporary file I created while converting to Eigen. I just removed it!

Copy link
Collaborator

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

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

@aabrown100-git Lots of good work here!

CMakeLists.txt Outdated Show resolved Hide resolved
Code/Source/solver/mat_fun.h Outdated Show resolved Hide resolved
Code/Source/solver/mat_fun.h Outdated Show resolved Hide resolved
Code/Source/solver/mat_fun.h Outdated Show resolved Hide resolved
Code/Source/solver/mat_fun.h Show resolved Hide resolved
Code/Source/solver/mat_models.cpp Outdated Show resolved Hide resolved
Code/Source/solver/mat_models.cpp Show resolved Hide resolved
Code/Source/solver/mat_models.cpp Outdated Show resolved Hide resolved
Code/Source/solver/sv_struct.cpp Show resolved Hide resolved
tests/unitTests/test.h Show resolved Hide resolved
@aabrown100-git
Copy link
Collaborator Author

Looks like -march=native causes an issue on GitHub Ubuntu machines; the integration tests timed out when I included -march=native in CMakeLists.txt, but passed in the most recent commit when I removed -march=native. From the timing tests I performed, there is some improvement with -march=native, but it's not huge. Also, here's a relevant discussion why -march=native is not enabled by default. For these reasons, I'll remove -march=native from CMakeLists.txt. Perhaps we can revisit this once @zasexton completes the Eigen backend for Arrays and learns more about Eigen performance.

…pressions. Avoid overloaded function and improve readability
…nd EigenTensor aliases to Matrix and Tensor. Consolidate double_dot_product functions for tensors. Rename get_() functions to compute_(). Fix throw runtime_error bug for invalid material models.
Copy link
Collaborator

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

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

It all looks good to me!

@aabrown100-git
Copy link
Collaborator Author

Awesome, I will merge now. Thanks @ktbolt and @mrp089!

@aabrown100-git aabrown100-git merged commit 675b02b into SimVascular:main Dec 28, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restructure material models with Eigen library
3 participants