[FEM.Elastic] Introduce generic element-agnostic elastic FEM force field#5882
[FEM.Elastic] Introduce generic element-agnostic elastic FEM force field#5882alxbilger wants to merge 16 commits intosofa-framework:masterfrom
Conversation
|
[ci-build][with-all-tests] |
th-skam
left a comment
There was a problem hiding this comment.
I only touched some easy to read files. Will try to take a better look in the next days.
But I really like the overall work.
...nent/SolidMechanics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/VecView.h
Show resolved
Hide resolved
|
This PR deserves the attention of all @sofa-framework/reviewers |
| for (sofa::Size l = 0; l < spatial_dimensions; ++l) | ||
| { | ||
| const auto ijkl = callable(i, j, k, l); | ||
| const auto jikl = callable(i, j, k, l); |
There was a problem hiding this comment.
| const auto jikl = callable(i, j, k, l); | |
| const auto jikl = callable(j, i, k, l); |
| return gradient; | ||
| } | ||
|
|
||
| static constexpr std::array<QuadraturePointAndWeight, 4> quadraturePoints() |
There was a problem hiding this comment.
Shouldn't we be using 8-point rule for hexas? And similarly 4-point rule for quads instead of 3 in the respective file?
I guess it's a performance/approximative trade-off but if the work coming from Elasticity is to be used for didacticism, perhaps we should have some comment here and mention that it is approximative.
...EM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/rotations/PolarDecomposition.h
Outdated
Show resolved
Hide resolved
| * @param youngModulus The Young's modulus of the material, representing its stiffness. | ||
| * @param poissonRatio The Poisson's ratio of the material, describing its deformation behavior. | ||
| * @return The isotropic elasticity tensor represented as an object of | ||
| * `ElasticityTensor<DataTypes>`. |
There was a problem hiding this comment.
| * `ElasticityTensor<DataTypes>`. | |
| * `FullySymmetric4Tensor<DataTypes>`. |
...lidMechanics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/KroneckerDelta.h
Outdated
Show resolved
Hide resolved
...lidMechanics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/LameParameters.h
Show resolved
Hide resolved
...anics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/MajorSymmetric4Tensor.h
Outdated
Show resolved
Hide resolved
...anics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/MajorSymmetric4Tensor.h
Outdated
Show resolved
Hide resolved
...echanics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/StrainDisplacement.h
Outdated
Show resolved
Hide resolved
| auto row = spatial_dimensions; | ||
| for (sofa::Size i = 0; i < spatial_dimensions; ++i) | ||
| { | ||
| for (sofa::Size j = i + 1; j < spatial_dimensions; ++j) | ||
| { | ||
| B(row, ne * spatial_dimensions + i) = gradientShapeFunctions[ne][j]; | ||
| B(row, ne * spatial_dimensions + j) = gradientShapeFunctions[ne][i]; | ||
| ++row; | ||
| } | ||
| } |
There was a problem hiding this comment.
| auto row = spatial_dimensions; | |
| for (sofa::Size i = 0; i < spatial_dimensions; ++i) | |
| { | |
| for (sofa::Size j = i + 1; j < spatial_dimensions; ++j) | |
| { | |
| B(row, ne * spatial_dimensions + i) = gradientShapeFunctions[ne][j]; | |
| B(row, ne * spatial_dimensions + j) = gradientShapeFunctions[ne][i]; | |
| ++row; | |
| } | |
| } | |
| for (sofa::Size i = 0; i < spatial_dimensions; ++i) | |
| { | |
| for (sofa::Size j = i + 1; j < spatial_dimensions; ++j) | |
| { | |
| const auto row = tensorToVoigtIndex<DataTypes>(i, j); | |
| B(row, ne * spatial_dimensions + i) = gradientShapeFunctions[ne][j]; | |
| B(row, ne * spatial_dimensions + j) = gradientShapeFunctions[ne][i]; | |
| } | |
| } |
There's a mismatch here between what's in VoigtNotation.h which states:
For 3D: (0,0)->0, (1,1)->1, (2,2)->2, (1,2)->3, (0,2)->4, (0,1)->5
But if you unroll the loop here, you would get
row = 3 -> (0,1), 4 -> (0,2), 5 -> (1,2) so 3 and 5 from voigt are swapped.
So the B^T C B in ElementStiffnessMatrix.h mixes them up.
And I assume we're getting away with it because we're dealing with isotropic problems but if the same code is to be used in the future we won't. Better to have a test to verify this now.
|
|
||
| const auto H = P.multTranspose(Q); | ||
|
|
||
| sofa::helper::Decompose<sofa::Real_t<DataTypes>>::polarDecomposition_stable(H, rotationMatrix); |
There was a problem hiding this comment.
What's the diff with the regular one ? Why keep both ?
There was a problem hiding this comment.
No idea. As usual, I want to keep compatibility with legacy SOFA. TetrahedronFEMForceField offers the option to select both polar decomposition. SVD uses the stable polar decomposition.
| const auto klij = callable(k, l, i, j); | ||
| const auto ijlk = callable(i, j, l, k); |
There was a problem hiding this comment.
Isn't the second minor symetry missing ? That being said, testing the first one and the major one seems sufficient.
.../SolidMechanics/FEM/Elastic/src/sofa/component/solidmechanics/fem/elastic/impl/MatrixTools.h
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
This should be purely added to the Core Matrix API
|
I insist on the fact that this PR deserves the attention of all @sofa-framework/reviewers |
Coming from https://github.com/alxbilger/Elasticity
TODO:
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if