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.
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
[GeoMechanicsApplication] A 3D elastic constitutive law is lacking in GeoMechanicsApplication #12816
[GeoMechanicsApplication] A 3D elastic constitutive law is lacking in GeoMechanicsApplication #12816
Changes from 4 commits
2ce1e36
c6e43b4
27e4090
e927d15
82fe15f
9889c47
8ce1180
b683398
95893d9
b081e1e
96954f7
4077a4a
7477196
2d19354
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I'm doubting a bit about the name of the class, the plane strain counterpart is just called PlaneStrain, but I'm not sure if ThreeDimensional would be descriptive enough.
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.
I'm in favor of renaming this class to
ThreeDimensional
. Needless to say that we then also need to rename this header file as well as the corresponding implementation file.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.
For me it is.
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.
What do @rfaasse and @WPK4FEM think about
ElasticIsotropic3D
and to changeConstitutiveLawDimension
toConstitutiveLawType
? For example, there isGeoLinearElasticPlaneStress2DLaw
which fits this structure perfectly usingPlaneStress
likePlaneStrain
. Another candidate isElasticIsotropicK03DLaw
. Thank you.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.
Changed to
ThreeDimensional
. However, looking to a discussion on my suggestion. ;)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.
Perhaps we should find out what Stenberg shear stabilization is and whether it is applicable for elasticity. We are moving this around, but we are not really using it.
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.
Looked at Stenberg stabilization. Only seems applicable for thick shell formulation used in Structural Mechanics Applications. I consider it to be part of another PR to limit its use to meaningful use in GeoMechanics.
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.
The implementation reads like SetFeatureOptions.
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.
Indeed ;) It is used across Kratos.
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.
I think this could be re-written a bit more (as you already started doing so): I'd propose something like:
This class defines an incremental linear elastic constitutive model for plane strain and 3D cases.
I don't think that 'small deformation' is really accurate (or has anything to do with the linear elastic model), but please correct me if I'm wrong (tagging you @WPK4FEM, to hear your opinion on this as well)
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.
To stress the incremental nature of the implementation, we could replace "small deformation" with "incremental". The formulation can be used both for small deformation and large deformation.
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.
replaced
small deformation
withincremental
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.
Rotting comment, was already wrong before the current change.
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.
corrected
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.
Same wrong 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.
corrected
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.
I think we should have the 'incremental' specifier in the registered name (maybe GeoIncrementalLinearElastic3DLaw?), but it's good to also have @WPK4FEM's opinion on this)
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.
I agree to have "Incremental" in the registered name.
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.
I share Richards point of view.
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.
Added
Incremental