Simplify creating and unrolling symmetric tensors #5640
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gassmoeller
force-pushed
the
add_unroll_function_symmetric_tensor
branch
from
fbc07cc
to
cb65ed1
Compare
This was referenced May 23, 2024
|
I think it is great to simplify and clean up the code based like this. |
bangerth
reviewed
May 29, 2024
include/aspect/utilities.h
Outdated
Comment on lines
1272
to
1275
| for (unsigned int i=0; i < SymmetricTensor<2,dim>::n_independent_components; ++i) | ||
| values[i] = tensor[SymmetricTensor<2,dim>::unrolled_to_component_indices(i)]; |
There was a problem hiding this comment.
Can you add
Assert (values.size() == ...n_indep_comp, ...)
?
Same for the other function.
gassmoeller
force-pushed
the
add_unroll_function_symmetric_tensor
branch
5 times, most recently
from
5ada0f2
to
b3b9c59
Compare
|
Ok, I think I addressed all comments. The new versions have more expressive names and use iterators instead of an ArrayView, which makes them more flexible to use with different data containers. Feel free to take a look and when this is merged I can prepare a similar path for deal.II. |
gassmoeller
force-pushed
the
add_unroll_function_symmetric_tensor
branch
from
b3b9c59
to
5999d1c
Compare
bangerth
approved these changes
May 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Something I noticed while reviewing #4370. We have quite a number of places in the code where we create Tensors from arrays or vectors, or unroll Tensors into arrays or vectors. We could simplify this using Tensor constructors, and appropriate
unrollfunctions. Such functions are already provided by deal.II for theTensorclass, but they dont exist forSymmetricTensor. I here created small utility functions forSymmetricTensor, but of course it would be nice to use a version inside deal.II. Would that be of interest?I can extend this PR to simplify a lot of places in ASPECT with the utility functions, but I first wanted to ask for feedback if the current interface is correct and if this is useful overall.