-
Notifications
You must be signed in to change notification settings - Fork 261
[Structural] Use Gauss rule in solid shell element #13421
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
Conversation
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.
You cannot simply use standard gauss points. The model has specific integration and can be manually replaced, I will do, but please don't use the standard ones
Sorry, I wasn't aware of this. Feel free to pirate the branch and the PR. |
I just checked and this is more difficult I remembered... There are two options, or I harcode the shape functions and gradients in the element or I generate a new prism deriving from the base one changing the integration methods. |
Taking into account that seems to be something very specific, I'd go for option one. I also note that this would be much easier too. |
The easier and nastier would be copy paste the geometry :P, but the KISS
would be to hardcode the gradients and jacobians. It will take some time, I
hope there is no hurry.
El mar., 13 may. 2025 7:48, Rubén Zorrilla ***@***.***>
escribió:
… *rubenzorrilla* left a comment (KratosMultiphysics/Kratos#13421)
<#13421 (comment)>
I just checked and this is more difficult I remembered... There are two
options, or I harcode the shape functions and gradients in the element or I
generate a new prism deriving from the base one changing the integration
methods.
Taking into account that seems to be something very specific, I'd go for
option one. I also note that this would be much easier too.
—
Reply to this email directly, view it on GitHub
<#13421 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEYQZAH5NCK33TWNZF43ODD26GBURAVCNFSM6AAAAAB46O5FDKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNZVGEZDCMJTHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
There's no hurry at all. However, I note that a student of us (@AlejandroCornejo) needs this to be finally fixed to advance with his thesis so we might eventually require to flag the SolidShell tests as expected fail and move forward. |
It would take me a couple of days, not weeks or months. I would not deactivate tests. BTW, you, me and @AlejandroCornejo we have done our thesis with branches. How can be so important to merge this?, the integrations schemes are basically enums... |
She is an undergraduate student who works with releases. I'd like to avoid creating a release just for her... Nevertheless, if you manage to do it in a couple of days there shouldn't be any problem. Thanks for taking care of it. |
…larations and enhance documentation - Replaced typedefs with using declarations for improved readability and modern C++ style. - Enhanced documentation for constructors, methods, and member variables to clarify functionality and usage. - Corrected typos in comments and improved descriptions for enums and methods. - Added static methods for integration points and shape functions to streamline calculations.
Should be OK now. |
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.
@loumalouomega you shouldn't approve a PR you contributed to.
Also, please do an actual review on something you approve.
...ucturalMechanicsApplication/custom_elements/solid_elements/solid_shell_element_sprism_3D6N.h
Outdated
Show resolved
Hide resolved
...turalMechanicsApplication/custom_elements/solid_elements/solid_shell_element_sprism_3D6N.cpp
Outdated
Show resolved
Hide resolved
...turalMechanicsApplication/custom_elements/solid_elements/solid_shell_element_sprism_3D6N.cpp
Outdated
Show resolved
Hide resolved
…nsistency - Removed unused IntegrationMethod type alias. - Reformatted constructor signatures for better readability. - Updated parameter names for consistency (e.g., `ThisNodes` to `rNodes`). - Added a new method to return the integration method used. - Improved documentation comments for clarity. - Corrected typos in comments and variable names. - Enhanced code structure by grouping related methods and variables.
@matekelemen this is IMO ready to be merged. The changes amend all the current inconveniences of the solid-shell element. |
I'm on holiday till the end of next week, so feel free to override my block. |
I think it is you who must override it |
Yes. We need you to approve it. |
I thought the TC could ignore other collaborators' blocks. Fine, I'll take a look in the next 2 days, though I might not be the expert here. Maybe @AlejandroCornejo or @sunethwarna would be better qualified. |
@loumalouomega is the expert here. It's a very specific element with lot of particularities inherent to the formulation that he developed during his PhD. From the @KratosMultiphysics/technical-committee we will acknowledge you unblocking this so we can proceed with #13085. Note that the changes in here do not affect anything aside of the sprism element. Also note that the changes in the geometries are those required after our decision on the quadratures issue (also included by myself in #13085). |
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.
Fine.
I'll just stress my dislike for, and the dangers of, self-approved changes ending up so easily in master.
As @rubenzorrilla said this element is super specific and developed in the context of my thesis, and these changes are part of a bigger PR and this PR was blocking it. I am not doing this sistematically. |
No worries. Yours is a very fair point. However, I think we can consider this a very specific case. Also note that the original implementation was mines but we required @loumalouomega to step in because the aforementioned reasons. Thanks for the quick out-of-office approval. |
📝 Description
In line to what has been discussed in #13085, this PR targets arranging the KratosCore integration rules. Specifically, this PR removes the unique use of the problematic extended Gauss rules, that is from the solid shell element.
@loumalouomega I substituted the extended Gauss by the standard ones. Once we have the Lobatto ones properly implemented, we can update it to these if you consider it more appropriate.
PD: el put* elemento maligno ataca de nuevo.