sofa
sofa copied to clipboard
[SofaSimpleFem] Refactor TetrahedronFEMForceField for better readability
This is a tentative to refactor TetrahedronFEMForceField
for better readability.
All the FEM methods have been dispatched into dedicated files.
The methods are now classes sharing the same interface (polymorphism), called by the component.
Advantages:
-
TetrahedronFEMForceField
is smaller -
TetrahedronFEMForceField
had many similar codes (often identical). This is now avoided as all common code are in the base classes. - It is easier to read the differences between the different FEM methods.
- We could imagine having a factory of FEM methods for
TetrahedronFEMForceField
. Then, the component could be extended in plugins by adding more methods into the factory.
To do:
- [ ] Clean
- [ ] Move some class members from
TetrahedronFEMForceField
toTetrahedronFEMForceFieldImpl
- [ ] Restore initial performances
- [ ] Fix broken features such as drawing rotations
- [ ] If everyone agrees, remove the assembled methods. It adds complexity while the resulted matrix is not used.
- [ ] Adapt (if necessary) the derived classes (multithreading, CUDA etc)
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
- it builds with SUCCESS for all platforms on the CI.
- it does not generate new warnings.
- it does not generate new unit test failures.
- it does not generate new scene test failures.
- it does not break API compatibility.
- it is more than 1 week old (or has fast-merge label).
arg... good input but it is not going to help my work on merging back this class with TetrahedralCorotationalFEMForceField... I started the other way, by moving the "generic" methods into a TetrahedronFEMUtils
There is a lot of code in the .h. Would it be possible to have the same refactoring while keeping the headers clean.
@damienmarchal @epernod This PR is a WIP. I did not finish it on purpose. The goal was to show a new organisation of the code and discuss about it. Thanks for having reviewed it. It means we can discuss about it on Wednesday ;)
Discussion on the improvement of FE codes in SOFA, Caribou plugin etc. Clarifying the FEM codes is really good, using utility classes (static functions) can help the reading, understanding of the physical model. The loss of performance in addDForce must be investigated → keep wip
Some bench to see the perf loss, on examples/Components/forcefield/TetrahedronFEMForcefield (with only one beam with small method):
master: TetrahedronFEMForceFieldScene>/1024 4504 ms
PR: TetrahedronFEMForceFieldScene>/1024 5735 ms
+~25% slower... 😥
Some bench to see the perf loss, on examples/Components/forcefield/TetrahedronFEMForcefield (with only one beam with small method): master:
TetrahedronFEMForceFieldScene>/1024 4504 ms
PR:TetrahedronFEMForceFieldScene>/1024 5735 ms
+~25% slower... 😥
@fredroy I don't remember such a high difference. I don't think I worked on small deformation. Have you tried large def?
Some bench to see the perf loss, on examples/Components/forcefield/TetrahedronFEMForcefield (with only one beam with small method): master:
TetrahedronFEMForceFieldScene>/1024 4504 ms
PR:TetrahedronFEMForceFieldScene>/1024 5735 ms
+~25% slower... 😥@fredroy I don't remember such a high difference. I don't think I worked on small deformation. Have you tried large def?
For big deformation:
master: TetrahedronFEMForceFieldScene>/1024 6717 ms
PR: TetrahedronFEMForceFieldScene>/1024 8100 ms
so ~20% slower
Additional note, these benches are done using clang(13) on linux (and in a VM)
Update (as bench on scenes is unreliable)
- Linux/clang:
15.5sec
(master) vs16.5sec
(pr) - Windows/cl:
17.5sec
(master) vs18.2sec
(pr)
Much less difference after all