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] Make name of constraints more explicit #4302

Merged
merged 35 commits into from
Dec 20, 2023

Conversation

bakpaul
Copy link
Contributor

@bakpaul bakpaul commented Nov 17, 2023

This PR is a first step for the issue #3952.

I modified the names of the constraints following the rule that we agreed upon. Still need to change spring force-fields that can behave as penalties.

The changes where straight, not more though than adding the type into the name was done. If any name doesn't fit your standard, feel free to suggest any changes.

This PR also add a componentRenamed mechanism that creates an alias automatically for renamed objects.

TODO:

  • Edit Sofa/framework/Helper/src/sofa/helper/ComponentChange.cpp
    • Add a API in ComponentChange.cpp for Renamed component (avoiding to add an alias manually in the class)
  • Move the C++ alias in the old header
  • Change name of projective constraints strating by ProjectOn

@bakpaul bakpaul added refactoring Refactor code STC#16 Tasks for STC#16 coding sprint labels Nov 17, 2023
@bakpaul bakpaul added this to the v23.12 milestone Nov 17, 2023
@bakpaul bakpaul added topic for next dev-meeting PR to be discussed in sofa-dev meeting pr: status to review To notify reviewers to review this pull-request labels Nov 20, 2023
@bakpaul bakpaul 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 Nov 21, 2023
@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 Nov 21, 2023
@bakpaul
Copy link
Contributor Author

bakpaul commented Nov 21, 2023

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

@bakpaul
Copy link
Contributor Author

bakpaul commented Nov 21, 2023

I am just not sure about the alias compatibility. It is for scene launching, but nothing guaranty that those aliases will one day be removed. Maybe the factory lacks a method addDeprecationAlias (or I am not aware of this) ?

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.

You need to edit Sofa/framework/Helper/src/sofa/helper/ComponentChange.cpp

@hugtalbot hugtalbot modified the milestones: v23.12, v24.06 Nov 22, 2023
@hugtalbot
Copy link
Contributor

hugtalbot commented Nov 22, 2023

Move the C++ alias in the old header (with the include of the new .h before)

Doc for lifecycle: https://www.sofa-framework.org/community/doc/contributing-to-sofa/lifecycle-mechanisms/#renaming-a-component

Maybe thinking about adding a API in ComponentChange.cpp for Renamed component (avoiding to add an alias manually in the class)

@fredroy fredroy 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 Nov 29, 2023
fredroy and others added 6 commits November 29, 2023 14:49
* [LinearSolver.Direct] Remove CSparse based solvers

* Remove csparse extlib

* Fetchable CSparseSolvers plugin

* Remove references of SparseLUSolver

* Remove references to SparseCholeskySolver

* Remove csparse

* Try to fix on Linux

* Update the compat

* [LinearSolver.Direct] Remove CSparse based solvers

* Fix direct solver
Co-authored-by: erik pernod <erik.pernod@gmail.com>
* Add cmake module for QGLViewer

* Support library name for apt packages

* Fix qglviewer cmake finder module to first seach for a config cmake file.

* Update cmake/Modules/FindQGLViewer.cmake

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
* Remove unlogical use of visitor

* Delete unused methods
@bakpaul
Copy link
Contributor Author

bakpaul commented Nov 29, 2023

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

@alxbilger alxbilger 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 Dec 6, 2023
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.

Almost good to go. Keep in mind that the documentation website will need to be updated accordingly.

@@ -32,10 +32,10 @@ namespace sofa::component::constraint::lagrangian::model
using namespace sofa::defaulttype;
using namespace sofa::helper;

int SlidingConstraintClass = core::RegisterObject("TODO-SlidingConstraint")
.add< SlidingConstraint<Vec3Types> >(true);
int SlidingLagrangianConstraintClass = core::RegisterObject("TODO-SlidingLagrangianConstraint")
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not really happy with this description...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes me neither, but in fact this is not the only component and it is not really in the scope of this pr...

applications/plugins/SofaCarving/examples/CarvingTool.scn Outdated Show resolved Hide resolved
@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 Dec 11, 2023
@bakpaul
Copy link
Contributor Author

bakpaul commented Dec 18, 2023

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

@bakpaul
Copy link
Contributor Author

bakpaul commented Dec 18, 2023

Finally I needed the changed introduced in the (now closed) PR #4324 to finalize the automatic alias creation mechanism. Now CI should not be crying anymore.

@bakpaul
Copy link
Contributor Author

bakpaul commented Dec 18, 2023

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

@alxbilger alxbilger 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 Dec 20, 2023
@hugtalbot hugtalbot merged commit 026a501 into sofa-framework:master Dec 20, 2023
10 checks passed
fredroy added a commit to fredroy/sofa that referenced this pull request Dec 21, 2023
)

* Added projective and lagrangian suffix

* Reported changes to Cuda plugin

* Modify rest of the code to avoid warning from compatibility layer

* Add compatibility layer for deprecated header in CUDA

* add alias for compatibility layer in scene construction

* Report name modification to all scenes

* Revert Compat modification...

* [LinearAlgera, Core] Fix linking with LTO on MacOS/Clang (sofa-framework#4293)

Fix linking with LTO on macos

* [LinearSolver] Remove CSparse-based linear solvers (sofa-framework#4258)

* [LinearSolver.Direct] Remove CSparse based solvers

* Remove csparse extlib

* Fetchable CSparseSolvers plugin

* Remove references of SparseLUSolver

* Remove references to SparseCholeskySolver

* Remove csparse

* Try to fix on Linux

* Update the compat

* [LinearSolver.Direct] Remove CSparse based solvers

* Fix direct solver

* [GUI.Qt] Minor single-line cleaning (sofa-framework#4308)

Co-authored-by: erik pernod <erik.pernod@gmail.com>

* [Sofa.GUI.Qt] Add cmake module for QGLViewer (sofa-framework#4290)

* Add cmake module for QGLViewer

* Support library name for apt packages

* Fix qglviewer cmake finder module to first seach for a config cmake file.

* Update cmake/Modules/FindQGLViewer.cmake

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

* [MultiThreading] Avoid Static Initialization Order Fiasco (sofa-framework#4307)

* [Core] Minor clean of DefaultAnimationLoop  (sofa-framework#4314)

* Remove unlogical use of visitor

* Delete unused methods

* [LinearSystem] Speedup computation of Jacobian matrices  (sofa-framework#4317)

* [LinearSystem] Speedup computation of Jacobian matrices in case both mstates are the same

* minor reformat to align calls to computeJacobiansFrom

* [SofaCUDA] FIX compilation SofaCUDA along with SparseGrid with Cuda12 (sofa-framework#4319)

* FIX compilation SofaCUDA along with SparseGrid with Cuda12

* Use a multiplication instead

* [Common] Add message to make the fetch mechanism less hidden (sofa-framework#4310)

* Add message to make it more clear

* slight modifications to be even more clear and add an error the dependency cannot be met

* [Collections] Remove reference to non-existing SofaSimulation (sofa-framework#4320)

remove ref to non-existent sofasimu

Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>

* Remove repeating preffix from projective constraints

* Moved alias to original header

* Changed deprecation date

* add component change entry

* Change name following discussion with Hugo

* Change remaining scene with FixedConstraint

* remove last occurences in py files

* Changed the message display of sceneCheckUsingAlias when aliased component is in component change

* Fix factory

* Add alias creation when object in componentRenamed in factory

* Remove precedently added aliases

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: erik pernod <erik.pernod@gmail.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
hugtalbot added a commit to hugtalbot/sofa that referenced this pull request Jan 16, 2024
)

* Added projective and lagrangian suffix

* Reported changes to Cuda plugin

* Modify rest of the code to avoid warning from compatibility layer

* Add compatibility layer for deprecated header in CUDA

* add alias for compatibility layer in scene construction

* Report name modification to all scenes

* Revert Compat modification...

* [LinearAlgera, Core] Fix linking with LTO on MacOS/Clang (sofa-framework#4293)

Fix linking with LTO on macos

* [LinearSolver] Remove CSparse-based linear solvers (sofa-framework#4258)

* [LinearSolver.Direct] Remove CSparse based solvers

* Remove csparse extlib

* Fetchable CSparseSolvers plugin

* Remove references of SparseLUSolver

* Remove references to SparseCholeskySolver

* Remove csparse

* Try to fix on Linux

* Update the compat

* [LinearSolver.Direct] Remove CSparse based solvers

* Fix direct solver

* [GUI.Qt] Minor single-line cleaning (sofa-framework#4308)

Co-authored-by: erik pernod <erik.pernod@gmail.com>

* [Sofa.GUI.Qt] Add cmake module for QGLViewer (sofa-framework#4290)

* Add cmake module for QGLViewer

* Support library name for apt packages

* Fix qglviewer cmake finder module to first seach for a config cmake file.

* Update cmake/Modules/FindQGLViewer.cmake

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

* [MultiThreading] Avoid Static Initialization Order Fiasco (sofa-framework#4307)

* [Core] Minor clean of DefaultAnimationLoop  (sofa-framework#4314)

* Remove unlogical use of visitor

* Delete unused methods

* [LinearSystem] Speedup computation of Jacobian matrices  (sofa-framework#4317)

* [LinearSystem] Speedup computation of Jacobian matrices in case both mstates are the same

* minor reformat to align calls to computeJacobiansFrom

* [SofaCUDA] FIX compilation SofaCUDA along with SparseGrid with Cuda12 (sofa-framework#4319)

* FIX compilation SofaCUDA along with SparseGrid with Cuda12

* Use a multiplication instead

* [Common] Add message to make the fetch mechanism less hidden (sofa-framework#4310)

* Add message to make it more clear

* slight modifications to be even more clear and add an error the dependency cannot be met

* [Collections] Remove reference to non-existing SofaSimulation (sofa-framework#4320)

remove ref to non-existent sofasimu

Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>

* Remove repeating preffix from projective constraints

* Moved alias to original header

* Changed deprecation date

* add component change entry

* Change name following discussion with Hugo

* Change remaining scene with FixedConstraint

* remove last occurences in py files

* Changed the message display of sceneCheckUsingAlias when aliased component is in component change

* Fix factory

* Add alias creation when object in componentRenamed in factory

* Remove precedently added aliases

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: erik pernod <erik.pernod@gmail.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
bakpaul added a commit to bakpaul/sofa that referenced this pull request Feb 23, 2024
)

* Added projective and lagrangian suffix

* Reported changes to Cuda plugin

* Modify rest of the code to avoid warning from compatibility layer

* Add compatibility layer for deprecated header in CUDA

* add alias for compatibility layer in scene construction

* Report name modification to all scenes

* Revert Compat modification...

* [LinearAlgera, Core] Fix linking with LTO on MacOS/Clang (sofa-framework#4293)

Fix linking with LTO on macos

* [LinearSolver] Remove CSparse-based linear solvers (sofa-framework#4258)

* [LinearSolver.Direct] Remove CSparse based solvers

* Remove csparse extlib

* Fetchable CSparseSolvers plugin

* Remove references of SparseLUSolver

* Remove references to SparseCholeskySolver

* Remove csparse

* Try to fix on Linux

* Update the compat

* [LinearSolver.Direct] Remove CSparse based solvers

* Fix direct solver

* [GUI.Qt] Minor single-line cleaning (sofa-framework#4308)

Co-authored-by: erik pernod <erik.pernod@gmail.com>

* [Sofa.GUI.Qt] Add cmake module for QGLViewer (sofa-framework#4290)

* Add cmake module for QGLViewer

* Support library name for apt packages

* Fix qglviewer cmake finder module to first seach for a config cmake file.

* Update cmake/Modules/FindQGLViewer.cmake

Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>

* [MultiThreading] Avoid Static Initialization Order Fiasco (sofa-framework#4307)

* [Core] Minor clean of DefaultAnimationLoop  (sofa-framework#4314)

* Remove unlogical use of visitor

* Delete unused methods

* [LinearSystem] Speedup computation of Jacobian matrices  (sofa-framework#4317)

* [LinearSystem] Speedup computation of Jacobian matrices in case both mstates are the same

* minor reformat to align calls to computeJacobiansFrom

* [SofaCUDA] FIX compilation SofaCUDA along with SparseGrid with Cuda12 (sofa-framework#4319)

* FIX compilation SofaCUDA along with SparseGrid with Cuda12

* Use a multiplication instead

* [Common] Add message to make the fetch mechanism less hidden (sofa-framework#4310)

* Add message to make it more clear

* slight modifications to be even more clear and add an error the dependency cannot be met

* [Collections] Remove reference to non-existing SofaSimulation (sofa-framework#4320)

remove ref to non-existent sofasimu

Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>

* Remove repeating preffix from projective constraints

* Moved alias to original header

* Changed deprecation date

* add component change entry

* Change name following discussion with Hugo

* Change remaining scene with FixedConstraint

* remove last occurences in py files

* Changed the message display of sceneCheckUsingAlias when aliased component is in component change

* Fix factory

* Add alias creation when object in componentRenamed in factory

* Remove precedently added aliases

---------

Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: erik pernod <erik.pernod@gmail.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
Co-authored-by: Hugo <hugo.talbot@sofa-framework.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: status ready Approved a pull-request, ready to be squashed refactoring Refactor code STC#16 Tasks for STC#16 coding sprint topic for next dev-meeting PR to be discussed in sofa-dev meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants