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

[Constraint.Lagrangian] Add fixed lagrangian constraint #4646

Merged

Conversation

bakpaul
Copy link
Contributor

@bakpaul bakpaul commented Apr 11, 2024

Add a lagrangian constraint working the same way as FixedProjectiveConstraint.
Added Vec3 and Rigid3 specialization.


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).

@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request pr: new feature Implement a new feature pr: highlighted in next release Highlight this contribution in the notes of the upcoming release labels Apr 11, 2024
@bakpaul bakpaul requested a review from hugtalbot April 11, 2024 08:01
Copy link
Contributor

@alxbilger alxbilger left a comment

Choose a reason for hiding this comment

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

Good initiative. We will have 3 different ways to fix points. 👍

@alxbilger
Copy link
Contributor

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

@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Apr 17, 2024
@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels Apr 24, 2024
@bakpaul
Copy link
Contributor Author

bakpaul commented Apr 24, 2024

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

Copy link
Contributor

@alxbilger alxbilger left a comment

Choose a reason for hiding this comment

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

If I want to be peaky, I would say that you don't provide code for instantiation for other types than Vec3Types and Rigid3Types. For example, FixedProjectiveConstraint also provides Vec2Types, Vec1Types, Vec6Types and Rigid2Types.

And if we look at Vec1Types specifically, can you explain the difference between FixedLagrangianConstraint<Vec1Types> and UniformLagrangianConstraint<Vec1Types>?

@bakpaul
Copy link
Contributor Author

bakpaul commented Apr 29, 2024

The difference lies in the fact that the UniformLagrangianConstraint is applied to all the model points. This assumption is important because thank's to this, it is able to use a block optimization when the data iterative is set to false. Here, it wouldn't be possible because we don't have the guaranty that the points ids will be contiguous.

As for the other specialization, what I did was to replicate the two specialization already available for the BilateralLagrangianConstraint. Actually I made sure to factorize as much as possible so that it would be easier for new specialization to pbe implemented. I could add them, but given that they didn't exist for the BilateralLagrangianConstraint, I don't know if they are really useful for anyone.

@fredroy fredroy force-pushed the add_fixed_lagrangian_constraint branch from a0b88e4 to a134fc0 Compare April 30, 2024 01:01
@hugtalbot hugtalbot added pr: status wip Development in the pull-request is still in progress and removed pr: status to review To notify reviewers to review this pull-request labels Apr 30, 2024
@bakpaul bakpaul added pr: status to review To notify reviewers to review this pull-request and removed pr: status wip Development in the pull-request is still in progress labels May 6, 2024
@bakpaul
Copy link
Contributor Author

bakpaul commented May 6, 2024

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

@fredroy fredroy added pr: status ready Approved a pull-request, ready to be squashed and removed pr: status to review To notify reviewers to review this pull-request labels May 7, 2024
@fredroy fredroy force-pushed the add_fixed_lagrangian_constraint branch from dc42be1 to feb52c5 Compare May 8, 2024 00:35
@fredroy fredroy merged commit 76f5b5d into sofa-framework:master May 8, 2024
10 of 11 checks passed
@bakpaul bakpaul added this to the v24.06 milestone May 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: highlighted in next release Highlight this contribution in the notes of the upcoming release pr: new feature Implement a new feature pr: status ready Approved a pull-request, ready to be squashed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants