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

Add elastic utilities #5284

Merged
merged 5 commits into from Jul 13, 2023
Merged

Conversation

MFraters
Copy link
Member

This pull request splits the tensor functions out of #4987 and #5116 and puts it in the utilities function in a new Tensors namespace.

@MFraters MFraters requested a review from bobmyhill July 13, 2023 00:43
@bobmyhill bobmyhill changed the title Add elasic utilities Add elastic utilities Jul 13, 2023
Copy link
Contributor

@bangerth bangerth left a comment

Choose a reason for hiding this comment

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

Very nice! In particular, I kept thinking that we ought to have tests for this, and then what do you know, there are tests at the bottom of the patch!

There are different ways of representing 3x3x3x3 tensors as 6x6 matrices, and the same applies to 3x3 tensors as vector of size 6. Voigt notation is one. Take a look at https://en.wikipedia.org/wiki/Voigt_notation.

I think it would be useful to annotate each function how exactly you want that representation to act -- perhaps by linking to the Wikipedia article.

include/aspect/utilities.h Outdated Show resolved Hide resolved


/**
* Rotate a 6x6 voigt matrix with an other 3D 4th
Copy link
Contributor

Choose a reason for hiding this comment

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

incomplet

* Rotate a 6x6 voigt matrix with an other 3D 4th
*/
SymmetricTensor<2,6>
rotate_6x6_matrix(const SymmetricTensor<2,6> &input_tensor, const Tensor<2,3> &rotation_tensor);
Copy link
Contributor

Choose a reason for hiding this comment

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

same here

rotate_6x6_matrix(const SymmetricTensor<2,6> &input_tensor, const Tensor<2,3> &rotation_tensor);

/**
* Transform a 4th order tensor into a 6x6 matrix
Copy link
Contributor

Choose a reason for hiding this comment

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

Period at the end of the sentence for all of the doc strings of all of these functions.

source/utilities.cc Outdated Show resolved Hide resolved
source/utilities.cc Outdated Show resolved Hide resolved
Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

A few more comments.

include/aspect/utilities.h Outdated Show resolved Hide resolved
include/aspect/utilities.h Outdated Show resolved Hide resolved
include/aspect/utilities.h Outdated Show resolved Hide resolved
include/aspect/utilities.h Outdated Show resolved Hide resolved
source/utilities.cc Outdated Show resolved Hide resolved
source/utilities.cc Outdated Show resolved Hide resolved
MFraters and others added 2 commits July 13, 2023 04:56
Co-authored-by: Rene Gassmoeller <rene.gassmoeller@mailbox.org>
Co-authored-by: Wolfgang Bangerth <bangerth@colostate.edu>
@MFraters
Copy link
Member Author

Thanks for the reviews. I have addressed you comments.

@bangerth
Copy link
Contributor

As mentioned earlier today, there seems to be a trend for functions of the form to_X, which omits the word convert_ and gets the type of the object being converted from the argument. An example is std::to_string.

@MFraters
Copy link
Member Author

I have changed the names

Copy link
Member

@gassmoeller gassmoeller left a comment

Choose a reason for hiding this comment

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

I dont mind the name change, but the new names do not seem consistent to me.

*/
SymmetricTensor<4,3>
rotate_full_stiffness_tensor(const Tensor<2,3> &rotation_tensor, const SymmetricTensor<4,3> &input_tensor);

Copy link
Member

Choose a reason for hiding this comment

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

Check the number of empty lines between all the functions in the header.

Comment on lines 1269 to 1276
transform_voigt_stiffness_vector_to_voigt_stiffness_matrix(const Tensor<1,21> &input_tensor);

/**
* Transform a 4th order full stiffness tensor into a 21D Voigt stiffness vector.
* See https://en.wikipedia.org/wiki/Voigt_notation for more info on the Voigt notation.
*/
Tensor<1,21>
transform_full_stiffness_tensor_to_voigt_stiffness_vector(const SymmetricTensor<4,3> &input);
Copy link
Member

Choose a reason for hiding this comment

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

why are these two not also to_voigt_stiffness_matrix and to_voigt_stiffness_vector?

@MFraters
Copy link
Member Author

sorry about that, I fixed it.

@gassmoeller gassmoeller merged commit 02dd578 into geodynamics:main Jul 13, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants