Conversation
| KRATOS_DEBUG_ERROR_IF_NOT(rStressVector.size() == msVectorSize) | ||
| << "PrincipalStresses can only be initialized with a vector of size " << msVectorSize | ||
| << ", got " << rStressVector.size() << std::endl; | ||
| std::ranges::copy(rStressVector, mValues.begin()); |
There was a problem hiding this comment.
An open question is if we would always like to sort the values in the vector when creating a 'principal stresses object'. What are your opinions @WPK4FEM, @avdg81, @mnabideltares?
There was a problem hiding this comment.
In principle my answer is yes. The desired ordering be found in the implementation of the MC with tension cut off material model.
However that would perhaps complicate the coping operations.
There was a problem hiding this comment.
I think it's a good idea to move the logic of the ordering/reordering of the stresses to this class (it seems it belongs there). However, since I have the impression it's quite tightly related to the way the Mohr Coulomb law works, I would like to do that in a separate PR and keep this one simple.
There was a problem hiding this comment.
An open question is if we would always like to sort the values in the vector when creating a 'principal stresses object'. What are your opinions @WPK4FEM, @avdg81, @mnabideltares?
I also agree that the sorting should be there, because there is code that relies on the fact that the components of the principal stress vector are sorted from largest to smallest. But indeed, such an extension can be done in a separate PR.
WPK4FEM
left a comment
There was a problem hiding this comment.
Hi Richard,
Indeed nice and short. The sorting thing will cause a lot of constraints on setting components. I don't know if it is wise to enforce that now.
Regards, Wijtze Pieter
| KRATOS_DEBUG_ERROR_IF_NOT(rStressVector.size() == msVectorSize) | ||
| << "PrincipalStresses can only be initialized with a vector of size " << msVectorSize | ||
| << ", got " << rStressVector.size() << std::endl; | ||
| std::ranges::copy(rStressVector, mValues.begin()); |
There was a problem hiding this comment.
In principle my answer is yes. The desired ordering be found in the implementation of the MC with tension cut off material model.
However that would perhaps complicate the coping operations.
applications/GeoMechanicsApplication/custom_constitutive/principal_stresses.hpp
Show resolved
Hide resolved
rfaasse
left a comment
There was a problem hiding this comment.
Thanks for the review, let me know whether you agree with the answers on the question/discussion!
applications/GeoMechanicsApplication/custom_constitutive/principal_stresses.hpp
Show resolved
Hide resolved
| KRATOS_DEBUG_ERROR_IF_NOT(rStressVector.size() == msVectorSize) | ||
| << "PrincipalStresses can only be initialized with a vector of size " << msVectorSize | ||
| << ", got " << rStressVector.size() << std::endl; | ||
| std::ranges::copy(rStressVector, mValues.begin()); |
There was a problem hiding this comment.
I think it's a good idea to move the logic of the ordering/reordering of the stresses to this class (it seems it belongs there). However, since I have the impression it's quite tightly related to the way the Mohr Coulomb law works, I would like to do that in a separate PR and keep this one simple.
avdg81
left a comment
There was a problem hiding this comment.
Hi Richard,
Thanks for preparing this class and making it similar to the SigmaTau class. I only have one very minor suggestion, which you can skip if no other changes are needed. 👍
...ions/GeoMechanicsApplication/tests/cpp_tests/custom_constitutive/test_principal_stresses.cpp
Outdated
Show resolved
Hide resolved
avdg81
left a comment
There was a problem hiding this comment.
Thanks for processing the review suggestions and resolving the one issue that was found by SonarQube. I feel this PR is good to go.
📝 Description
Very minimal class that wraps around a ublas vector to represent principal stresses