search

sofa-framework / sofa

Real-time multi-physics simulation with an emphasis on medical simulation.
https://www.sofa-framework.org
GNU Lesser General Public License v2.1
871 stars 297 forks source link

[SofaMiscFem] Proposal of FEM force field for Reissner-Mindlin Flat Shell Element #1745

Closed nhnhan92 closed 3 years ago

nhnhan92 commented 3 years ago

Here, I propose a raw version of Force Field for solving FEM model of Reissner-Mindlin Flat Shell Element (quad element, 5 degrees of freedom). Note that, at this current stage, I assume that 2 degrees of freedom related to rotational displacements of a node in x and y-axis (z is the normal axis of the element) are ignored for the purpose of my project. However, I also want to include these components in the calculation but have no clue. Furthermore, only the small displacement method is considered, I just wonder whether we can apply the co-rotational method to this type of element. Since I am not an expert in FEM, the calculation process and coding flow are inspired by many online materials and the TriangularFEMForceField, respectively. I believe it still got a lot of problems that I have not known yet, and also, this is my first contribution and PR, thus any helps from SOFA community will be sincerely appreciated PS: a scene that includes this FF is also provided and works perfectly (for me).

Needs merge of : https://github.com/sofa-framework/regression/pull/10

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

hugtalbot commented 3 years ago

Thank you @nhnhan92 for this very nice PR, we are reviewing it and will provide review comments ! :+1: very cool initiative anyway! :clap:

hugtalbot commented 3 years ago

Thanks again for this nice contribution @nhnhan92 Sorry I had prepared many similar feedback as @epernod so, some might be duplicated.

A more general remark, could you point us out the documentation you used to write this code please? Do you have an idea of when you would plan to consider rotations around the z axis? I also noticed in this paper typical tests for testing accuracy: https://sci-hub.se/10.1007/s00366-020-00974-4 it would be great to get a similar test for the continuous integration!

If this is too much questions, we can discuss about this live if you'd like

nhnhan92 commented 3 years ago

Thanks again for this nice contribution @nhnhan92 Sorry I had prepared many similar feedback as @epernod so, some might be duplicated.

A more general remark, could you point us out the documentation you used to write this code please? Do you have an idea of when you would plan to consider rotations around the z axis? I also noticed in this paper typical tests for testing accuracy: https://sci-hub.se/10.1007/s00366-020-00974-4 it would be great to get a similar test for the continuous integration!

If this is too much questions, we can discuss about this live if you'd like

Dear, @hugtalbot, Thank you for kind supports from you and @epernod. First, I implemented this FF mostly based on two following references:

  1. Book: "Structural Analysis with the Finite Element Method, Linear static, Vol. 2, chapter 8"
  2. And the most recent paper of our group: https://ieeexplore.ieee.org/document/9247533 On the other hand, at this stage, this FF fairly meets our expectations for the current project. I definitely want to improve it in the near future. However, FEM is not my field (I am basically an end-user) and I have been in the middle of something else, therefore, It is gonna take time for me to be able to complete it. I would love to cooperate with SOFA team to complete it. I'm sorry, but I'm quite unsure when you said rotation around z-axis (normal axis of the element?). Do you mean the consideration of rotating element around its normal axis? (i.e., co-rotational LARGE method (am I right?)). Right now, my priority is to take the rest two rotation dofs of a node (location rotations of around x and y direction) because, at this moment, I assumed they are zeros. But, so far, I have no idea how to include them Finally, if it is necessary, I am willing to have a face-to-face discussion as long as I am available. Thank you Best, Nhan
hugtalbot commented 3 years ago

Thank you for your update @nhnhan92 I added two reviews in the code.

Regarding the license, we need the SOFA header to be added in order to display the official license of the project. If this is not possible, an alternative would be to create a dedicated plugin which could have its own open-source license.

Finally, a last question would be : would it be possible to have a regression test comparing your material with a analytical/manufactured solution?

Thanks again for this nice piece of work.

hugtalbot commented 3 years ago

Any news @nhnhan92 ? Note that for my remark on the header, having your project name if fine for us. What matters for us is to have first the SOFA header (LGPL) up in the file. Would this be possible? Let me thank you again for this nice contribution.

nhnhan92 commented 3 years ago

Any news @nhnhan92 ? Note that for my remark on the header, having your project name if fine for us. What matters for us is to have first the SOFA header (LGPL) up in the file. Would this be possible? Let me thank you again for this nice contribution.

Hi @hugtalbot, Sorry for late response. I have recently been in the middle of some work (related to this project). Therefore, there is no progress so far, however, I plan to add the addKtoMatrix module to the FF and also try to improve coding stuffs (will be updated as soon as possible). On the other hand, I have discussed with my team member and advisor about the header. We agree with your above suggestion, but I just wonder is it possible for this FF to appear in both the SOFA source code (in the future) and our own under-developing plugin? we are not familiar with this procedure, sorry for that If it is okay, please write the name of the contributor as: JST/PRESTO Project: TouchIoT-Smart Tangible Sensing Enabler for Tactile Internet (Developer: Nhan Nguyen Huu) One last thing, from the previous comment, you mentioned the test. Could you please be more specific about the test? do you want to test the reliability of the FF (like coding, mathematical model, etc.)? We definitely want to certify that. Thank you for your and SOFA team effort to review our work. Best regards, Nhan

hugtalbot commented 3 years ago

Hey @nhnhan92 Thank you so much for your patience and your efforts regarding the headers. It is great to have homogeneous headers for the project, especially for people to use SOFA under one single LGPL license.

To my best knowledge, it is not really possible to have "two versions" of the code in SOFA and a plugin of yours. But here is what I would recommend: once this PR is merged, create a specific branch in your fork where you can implement / test new evolutions. Each time you think the time is appropriate to contribute it back, you can simply pull-request your branch. This is the power of Git!

Finally, regarding my remark about the test, I was thinkg about a C++ test (that we run on the CI, you can have a look to the C++ files ***_test.cpp. In this test, it would be ideal to have a numerical validation, assessing that the shell model is following the theoretical behavior (e.g. using method of manufactured solutions (MMS)) See: https://link.springer.com/article/10.1007/s00366-017-0572-4

Let us know what you think about it. We can make it step by step!

hugtalbot commented 3 years ago

A change is needed in SofaMiscFem_test/CMakeLists.txt by the way (as mentioned in my review)

nhnhan92 commented 3 years ago

Hey @nhnhan92 Thank you so much for your patience and your efforts regarding the headers. It is great to have homogeneous headers for the project, especially for people to use SOFA under one single LGPL license.

To my best knowledge, it is not really possible to have "two versions" of the code in SOFA and a plugin of yours. But here is what I would recommend: once this PR is merged, create a specific branch in your fork where you can implement / test new evolutions. Each time you think the time is appropriate to contribute it back, you can simply pull-request your branch. This is the power of Git!

Finally, regarding my remark about the test, I was thinkg about a C++ test (that we run on the CI, you can have a look to the C++ files _***test.cpp. In this test, it would be ideal to have a numerical validation, assessing that the shell model is following the theoretical behavior (e.g. using method of manufactured solutions (MMS)) See: https://link.springer.com/article/10.1007/s00366-017-0572-4

Let us know what you think about it. We can make it step by step!

Dear @hugtalbot, Thank you for your recommendation, we think that would be a proper option for our case. On the other hand, regarding the test, we want nothing more than that, but we are not familiar with the testing procedure. Therefore, we are willing to follow your guidance. Hope to hear your response soon. Best regards, Nhan

hugtalbot commented 3 years ago

Hi @nhnhan92 Here is what we propose: let's get this nice work merged as is. I will only add a regression scene to check that no alteration of the model occurs in the future. I will also write a short doc about it, and I will let you append any additional details on it.

A C++ test inspired from the article would be nice in another PR. For writing test, you can:

Is this fine for you?

nhnhan92 commented 3 years ago

Hi @nhnhan92 Here is what we propose: let's get this nice work merged as is. I will only add a regression scene to check that no alteration of the model occurs in the future. I will also write a short doc about it, and I will let you append any additional details on it.

A C++ test inspired from the article would be nice in another PR. For writing test, you can:

Is this fine for you?

Hi @hugtalbot, It is clear enough. I will take a look at the instruction and try to implement the .test file. I will inform you as soon as I get something new. Thank you again for your patient to help me. Best, Nhan

hugtalbot commented 3 years ago

Compilation has been fixed with master and the regression test has been added. Only the PR on regression must be merged and then reference in regression submodule udpated.

Let's get it merged tomorrow.

hugtalbot commented 3 years ago

[ci-build][with-all-tests]

hugtalbot commented 3 years ago

Owh, CI seems not happy. I will make sure to fix it (coming from the recent change in testing, to be propagated in regression)

hugtalbot commented 3 years ago

[ci-build][with-all-tests]

nhnhan92 commented 3 years ago

Hi @hugtalbot, I just got back from a long business term and realize that this PR has successfully been merged. Thank you for wonderful efforts of SOFA developer team in general and yours in particular. On the other hand, the contributor list has not added our project name as a contributor. I just wonder should I do it or just leave it to your team. looking forward your response Thank you again, Best regards, Nhan

hugtalbot commented 1 year ago

Hi @nhnhan92 just a curiosity, were you able to complete the implementation of the model? Are you still using it? We would be really glad to hear from you again ;)

nhnhan92 commented 1 year ago

Hi @hugtalbot, tks for the contact. Our team has still been using this FEM model for doing research. For now, this is sufficiently sastisfying for our purpose. However, we definitely want to improve it (in fact, a plan for doing it is actually discussed) in the near future for sure. I will make sure to notify you when we start doing it because we might need some technical help from your team. Thank you

hugtalbot commented 1 year ago

thanks for getting back to us @nhnhan92 :+1: