Rewrite the force transfer of applyJT functions in the DiscreteCosserat mapping#159
Open
peyronq1 wants to merge 1 commit intoSofaDefrost:masterfrom
Open
Rewrite the force transfer of applyJT functions in the DiscreteCosserat mapping#159peyronq1 wants to merge 1 commit intoSofaDefrost:masterfrom
peyronq1 wants to merge 1 commit intoSofaDefrost:masterfrom
Conversation
…pressed in the global frame. Small change required in the computation of the tangentExp in the case of bending and twist only (the base linear def along X should be 1).
hugtalbot
reviewed
Sep 16, 2025
Collaborator
hugtalbot
left a comment
hugtalbot
left a comment
There was a problem hiding this comment.
Nice to see you PRing @peyronq1 🔥
My 2 cents review, I let @adagolodjo look more closely
Comment on lines
+390
to
+392
| std::cout<<"Global F = " << global_F_Vec[s] <<" at s = " << s <<std::endl; | ||
| std::cout<<"temp = " << temp <<" at s = " << s <<std::endl; | ||
| std::cout<<"f = " << f <<" at s = " << s <<std::endl; |
Collaborator
There was a problem hiding this comment.
Possibly to remove or using msg API:
Suggested change
| std::cout<<"Global F = " << global_F_Vec[s] <<" at s = " << s <<std::endl; | |
| std::cout<<"temp = " << temp <<" at s = " << s <<std::endl; | |
| std::cout<<"f = " << f <<" at s = " << s <<std::endl; | |
| std::cout<<"Global F = " << global_F_Vec[s] <<" at s = " << s <<std::endl; | |
| std::cout<<"temp = " << temp <<" at s = " << s <<std::endl; | |
| std::cout<<"f = " << f <<" at s = " << s <<std::endl; |
Comment on lines
+399
to
+402
| /* this->computeCoAdjoint( | ||
| m_nodesExponentialSE3Vectors[index], | ||
| coAdjoint); // m_nodesExponentialSE3Vectors computed in apply | ||
| F_tot = coAdjoint * F_tot; | ||
| F_tot = coAdjoint * F_tot;*/ |
Comment on lines
+517
to
+525
| /*const auto _T = Frame(frame[childIndex].getCenter(), | ||
| frame[childIndex].getOrientation()); | ||
| Mat6x6 P_trans = (this->buildProjector(_T)); | ||
| P_trans.transpose(); | ||
|
|
||
| Mat6x6 co_adjoint; | ||
| this->computeCoAdjoint( | ||
| m_framesExponentialSE3Vectors[childIndex], | ||
| co_adjoint); // m_framesExponentialSE3Vectors[s] computed in apply | ||
| co_adjoint);*/ // m_framesExponentialSE3Vectors[s] computed in apply |
Comment on lines
+532
to
+537
| /*Vec6 local_F = | ||
| co_adjoint * P_trans * | ||
| valueConst; // constraint direction in local frame of the beam. | ||
| valueConst;*/ // constraint direction in local frame of the beam. | ||
|
|
||
| /*Vec3 f = matB_trans * temp * | ||
| local_F; // constraint direction in the strain space.*/ |
Comment on lines
+607
to
+613
| /*Mat6x6 coAdjoint; | ||
| this->computeCoAdjoint( | ||
| m_nodesExponentialSE3Vectors[i - 1], | ||
| coAdjoint); // m_nodesExponentialSE3Vectors computed in apply | ||
| CumulativeF = coAdjoint * CumulativeF; | ||
| // transfer to strain space (local coordinates) | ||
| // transfer to strain space (local coordinates)*/ | ||
|
|
alxbilger
requested changes
Sep 16, 2025
Member
alxbilger
left a comment
alxbilger
left a comment
There was a problem hiding this comment.
I recommend to clean your code:
- Remove commented code
- Remove the traces (
cout)
| } | ||
|
|
||
| template <class TIn1, class TIn2, class TOut> | ||
| auto DiscreteCosseratMapping<TIn1, TIn2, TOut>::getTildeMatrix(const Vec3 &u) |
Member
There was a problem hiding this comment.
Check if it is the same function than sofa::type::Mat::crossProductMatrix. If it is, you can remove this function and use crossProductMatrix instead.
Author
Thank you! But don't congratulate me yet haha. We are working on improvements with Younes but I am not sure he expected me to do a PR on the master branch. I will look at this with him. Thank you for the review! |
Collaborator
|
just a gentle ping @peyronq1 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrite the force transfer of applyJT functions considering forces expressed in the global frame. Small change required in the computation of the tangentExp in the case of bending and twist only (the base linear def along X should be 1).