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

[FastPR][Core] Missing Tetrahedra3D10 second derivatives #12082

Merged
merged 3 commits into from
Feb 20, 2024

Conversation

rubenzorrilla
Copy link
Member

📝 Description
Adding the shape functions second derivatives to the Tetrahedra3D10 geometry.

@rubenzorrilla rubenzorrilla added Kratos Core Incomplete FastPR This Pr is simple and / or has been already tested and the revision should be fast labels Feb 19, 2024
@rubenzorrilla rubenzorrilla self-assigned this Feb 19, 2024
@rubenzorrilla rubenzorrilla requested a review from a team as a code owner February 19, 2024 08:44
Copy link
Member

@loumalouomega loumalouomega left a comment

Choose a reason for hiding this comment

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

Please add test

@rubenzorrilla
Copy link
Member Author

Please add test

These are not tested in any geometry. Also note that there is nothing to test in here, values are hardcoded...

@loumalouomega
Copy link
Member

Please add test

These are not tested in any geometry.

Most geometries are linear, therefore this is not even existing

Also note that there is nothing to test in here, values are hardcoded...

In that case test should be easy...

@rubenzorrilla
Copy link
Member Author

Please add test

These are not tested in any geometry.

Most geometries are linear, therefore this is not even existing

I checked it for the quadratic ones featuring the implementation of this method.

Also note that there is nothing to test in here, values are hardcoded...

In that case test should be easy...

Easy but meaningless... Anyway, I'll try to figure out something.

@loumalouomega
Copy link
Member

In that case test should be easy...

Easy but meaningless... Anyway, I'll try to figure out something.

You may have hardcoded wrong, that happened to me several times. In any case I will approve and I leave up to to you add the test or not.

loumalouomega
loumalouomega previously approved these changes Feb 19, 2024
@rubenzorrilla
Copy link
Member Author

@loumalouomega rather than checking hardcoded values, which IMO makes no point at all, I decided to create a fake isoparametric tet (so we avoid the Jacobian transformation complexities) and check the derivatives values with a second order field, expected to be constant over the entire element.

@rubenzorrilla rubenzorrilla merged commit 52fc4e6 into master Feb 20, 2024
15 of 17 checks passed
@rubenzorrilla rubenzorrilla deleted the core/tetrahedra3d10-second-derivatives branch February 20, 2024 06:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FastPR This Pr is simple and / or has been already tested and the revision should be fast Incomplete Kratos Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants