Skip to content

[LinearAlgebra] Change access specifier for the method set #3834

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.

Sign up for GitHub

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

Jump to bottom
Merged
damienmarchal merged 2 commits into from
May 17, 2023
Merged

[LinearAlgebra] Change access specifier for the method set #3834

damienmarchal merged 2 commits into from
May 17, 2023

Conversation

@bakpaul
Copy link
Contributor

@bakpaul bakpaul commented May 3, 2023

Change access specifier for the method set. A public access is needed in SofaCUDASolver plugin.

Moreover, regardless of the reasons of this modification, the fact that this method is at the beginning of the class declaration without any access specifier before is not wanted. Even if it is a class which makes its access private by default, it lacks of readability.

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

Change access specifier for the method set. Needed in SofaCUDASolver …
fa6c02b 
…plugin. Regardless the resons of this modification, The fact that this method is at the begining of the class declaration without any access specifier befoe is not usual
@alxbilger alxbilger changed the title Change access specifier for the method set. [LinearAlgebra] Change access specifier for the method set. May 3, 2023
@alxbilger alxbilger added the pr: status to review To notify reviewers to review this pull-request label May 3, 2023
alxbilger
alxbilger approved these changes May 3, 2023
View reviewed changes
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.

According to the git history, the method set was initially disabled for this class. That is why it was private. See the commit that implemented the set method: 7a3ec34#diff-0dff698f57bb671243ce9650f2cc92c0a8a2b7ee53dc9af31a0d60ec6c314716

Since it is now implemented, it makes sense to make it public, just as in the base class

@hugtalbot hugtalbot added the pr: enhancement About a possible enhancement label May 7, 2023
@fredroy
Copy link
Contributor

fredroy commented May 8, 2023

Instead of moving the public: keyword , I would rather move the set() function in the other public section below with the other methods 😅

@hugtalbot hugtalbot added this to the v23.12 milestone May 10, 2023
@alxbilger
Copy link
Contributor

alxbilger commented May 11, 2023

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

@hugtalbot hugtalbot changed the title [LinearAlgebra] Change access specifier for the method set. [LinearAlgebra] Change access specifier for the method set May 17, 2023
@hugtalbot hugtalbot 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 17, 2023
@damienmarchal damienmarchal merged commit c6b725b into sofa-framework:master May 17, 2023
hugtalbot pushed a commit to hugtalbot/sofa that referenced this pull request Jun 1, 2023
@bakpaul @hugtalbot
[LinearAlgebra] Change access specifier for the method set (sofa-fram…
37e7221 
…ework#3834)

* Change access specifier for the method set. Needed in SofaCUDASolver plugin. Regardless the resons of this modification, The fact that this method is at the begining of the class declaration without any access specifier befoe is not usual

* Changed attributs/methods orders and remove repeated identifiers

---------

Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>
alxbilger added a commit that referenced this pull request Jun 14, 2023
@hugtalbot @alxbilger
@bakpaul @damienmarchal @fredroy @olivier-roussel
[all] Rename DefaultContactManager into CollisionResponse (#3891)
7dc85a8 
* Rename file DefaultContactManager into CollisionResponse

* Update Compat layer with the new class name

* udpate CMakeLists with new class name

* udpate CMakeLists with new class name keeping compat file

* update class name in the code

* propagate the change of C++ class name

* add compatibility file

* update all scenes with new component name CollisionResponse

* Apply suggestions from code review

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [LinearAlgebra] Change access specifier for the method set (#3834)

* Change access specifier for the method set. Needed in SofaCUDASolver plugin. Regardless the resons of this modification, The fact that this method is at the begining of the class declaration without any access specifier befoe is not usual

* Changed attributs/methods orders and remove repeated identifiers

---------

Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>

* [Spring] Remove unused dependencies (#3848)

* [StateContainer] Fix bug in dynamic data registration (#3783)

* [Sofa.Type] Replace the use of Matrix::s_identity (deprecated) by Matrix::Identity()

* [Sofa.Component.StateContainer] Fix missing registration of dynamic properties.


Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

* [SceneChecking] Add check when setting contactStiffness uselessly (#3843)

* [SceneChecking] Add check when setting contactStiffness uselessly

* Separate contribution in a separate function

* [test] Fix unit test on RestShapeSpringsForceField (#3864)

* [plugins] Remove fetching of SofaPython (#3855)

* [Constraint.Lagrangian] Activate the export of lambda forces by default (#3857)

* [all] Change variable name supportOnlySymmetricMatrix in MParams  (#3861)

* [all] Change variable name supportOnlySymmetricMatrix in MechanicalParams

* propagate the supportOnlySymmetricMatrix API change

* Add deprecation

---------

Co-authored-by: Alex Bilger <alexbilger0@gmail.com>

* [Constraint.Lagrangian] Add messages when no compliance is given (#3858)

* [Constraint.Lagrangian] Add messages when no compliance is given

* minor esthetic msg update

* [all] include base class inl file (#3865)

* [test] Fix failing unit test (#3876)

Introduced in #3858

* [LinearAlgebra] Pull Insimo's CompressedRowSparseMatrix into the main branch (#3515)

* WIP

* CRS: integration from issofa

* rename all bloc to blocks

* use MessagingAPI (and rename again bloc to block

* add matrix_bloc_traits for Vec

* fix compilation with CRS debug macros

* revert some useless changes

* add insimo as contributors

* fix compilation test by bringing back operators and MatrixExpr

* fix stuff about add()

* add compress() after clearing (fix some tests?)

* fix debug compilation

* clarify add() functions (indices, etc)

* apply insimo code and clean ups

* rename CRS for CRSGeneric

* apply insimo code for CRSMechanical (previous CRS) and clean ups

* add transitional header

* fix confusion about mul()

* bring back some changes from original version

* bring back BM:add with mat3 and various fixes

* fix gcc

* remove conversion warnings

* fix tests and stuff

* Use struct instead of class (+public)

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* remove diag/debug/trace api in crs

* remove now-useless CRS trace logger utility header

* remove some override warnings

* remove more warnings

* fix rebase

* refresh scene with MechanicalMatrixMapper

* change traits to try to match previous behavior

* fix missing include

* add explicit instanction for float type

* apply suggestion by using directly the correct type

* remove the touch trait

---------

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [SofaCUDA] Deprecated CudaTexture.h (#3869)

It is not used, and its content is deprecated in Cuda 12

* [examples] Fix unstable scene constantMomentum.scn (#3886)

* [SofaCUDA] Replace deprecated vector types (#3902)

* [Config] Fix cross-compilation for embedded external libs (#3870)

[Config] Fix cross-compilation by turning off cmake_find_root_path for embedded external libs

* [all] Deprecate unused verbose data (#3871)

* [all] Deprecate unused verbose data

* Add warning in parse method

* [contact] Add missing call to super init (#3884)

* [SolidMechanics] Use accessors & make geometrical data required in BFF (#3887)

[SolidMechanics] Use accessors & make geometrical data required in BeamFEMFF

* [SofaCUDA] No longer use deprecated texture references in HexaTLED (#3868)

* [SofaCUDA] Remove non-working scene

* Introduce example

* nodesPerElement

* DhC0, DhC1 and DhC2

* detJ

* Disp

* hourglass control

* preferredDirection

* Di1, Di2, Dv1, Dv2

* force coordinates

* Fi

* x, x0

* Ignore added scene on CI

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>
Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>
Co-authored-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alexbilger0@gmail.com>
Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
damienmarchal added a commit to CRIStAL-PADR/sofa that referenced this pull request Jun 28, 2023
@hugtalbot @alxbilger
@bakpaul @damienmarchal @fredroy @olivier-roussel
[all] Rename DefaultContactManager into CollisionResponse (sofa-frame…
957ee8c 
…work#3891)

* Rename file DefaultContactManager into CollisionResponse

* Update Compat layer with the new class name

* udpate CMakeLists with new class name

* udpate CMakeLists with new class name keeping compat file

* update class name in the code

* propagate the change of C++ class name

* add compatibility file

* update all scenes with new component name CollisionResponse

* Apply suggestions from code review

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [LinearAlgebra] Change access specifier for the method set (sofa-framework#3834)

* Change access specifier for the method set. Needed in SofaCUDASolver plugin. Regardless the resons of this modification, The fact that this method is at the begining of the class declaration without any access specifier befoe is not usual

* Changed attributs/methods orders and remove repeated identifiers

---------

Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>

* [Spring] Remove unused dependencies (sofa-framework#3848)

* [StateContainer] Fix bug in dynamic data registration (sofa-framework#3783)

* [Sofa.Type] Replace the use of Matrix::s_identity (deprecated) by Matrix::Identity()

* [Sofa.Component.StateContainer] Fix missing registration of dynamic properties.


Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

* [SceneChecking] Add check when setting contactStiffness uselessly (sofa-framework#3843)

* [SceneChecking] Add check when setting contactStiffness uselessly

* Separate contribution in a separate function

* [test] Fix unit test on RestShapeSpringsForceField (sofa-framework#3864)

* [plugins] Remove fetching of SofaPython (sofa-framework#3855)

* [Constraint.Lagrangian] Activate the export of lambda forces by default (sofa-framework#3857)

* [all] Change variable name supportOnlySymmetricMatrix in MParams  (sofa-framework#3861)

* [all] Change variable name supportOnlySymmetricMatrix in MechanicalParams

* propagate the supportOnlySymmetricMatrix API change

* Add deprecation

---------

Co-authored-by: Alex Bilger <alexbilger0@gmail.com>

* [Constraint.Lagrangian] Add messages when no compliance is given (sofa-framework#3858)

* [Constraint.Lagrangian] Add messages when no compliance is given

* minor esthetic msg update

* [all] include base class inl file (sofa-framework#3865)

* [test] Fix failing unit test (sofa-framework#3876)

Introduced in sofa-framework#3858

* [LinearAlgebra] Pull Insimo's CompressedRowSparseMatrix into the main branch (sofa-framework#3515)

* WIP

* CRS: integration from issofa

* rename all bloc to blocks

* use MessagingAPI (and rename again bloc to block

* add matrix_bloc_traits for Vec

* fix compilation with CRS debug macros

* revert some useless changes

* add insimo as contributors

* fix compilation test by bringing back operators and MatrixExpr

* fix stuff about add()

* add compress() after clearing (fix some tests?)

* fix debug compilation

* clarify add() functions (indices, etc)

* apply insimo code and clean ups

* rename CRS for CRSGeneric

* apply insimo code for CRSMechanical (previous CRS) and clean ups

* add transitional header

* fix confusion about mul()

* bring back some changes from original version

* bring back BM:add with mat3 and various fixes

* fix gcc

* remove conversion warnings

* fix tests and stuff

* Use struct instead of class (+public)

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* remove diag/debug/trace api in crs

* remove now-useless CRS trace logger utility header

* remove some override warnings

* remove more warnings

* fix rebase

* refresh scene with MechanicalMatrixMapper

* change traits to try to match previous behavior

* fix missing include

* add explicit instanction for float type

* apply suggestion by using directly the correct type

* remove the touch trait

---------

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [SofaCUDA] Deprecated CudaTexture.h (sofa-framework#3869)

It is not used, and its content is deprecated in Cuda 12

* [examples] Fix unstable scene constantMomentum.scn (sofa-framework#3886)

* [SofaCUDA] Replace deprecated vector types (sofa-framework#3902)

* [Config] Fix cross-compilation for embedded external libs (sofa-framework#3870)

[Config] Fix cross-compilation by turning off cmake_find_root_path for embedded external libs

* [all] Deprecate unused verbose data (sofa-framework#3871)

* [all] Deprecate unused verbose data

* Add warning in parse method

* [contact] Add missing call to super init (sofa-framework#3884)

* [SolidMechanics] Use accessors & make geometrical data required in BFF (sofa-framework#3887)

[SolidMechanics] Use accessors & make geometrical data required in BeamFEMFF

* [SofaCUDA] No longer use deprecated texture references in HexaTLED (sofa-framework#3868)

* [SofaCUDA] Remove non-working scene

* Introduce example

* nodesPerElement

* DhC0, DhC1 and DhC2

* detJ

* Disp

* hourglass control

* preferredDirection

* Di1, Di2, Dv1, Dv2

* force coordinates

* Fi

* x, x0

* Ignore added scene on CI

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>
Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>
Co-authored-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alexbilger0@gmail.com>
Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
bakpaul added a commit to bakpaul/sofa that referenced this pull request Feb 23, 2024
@hugtalbot @alxbilger
@bakpaul @damienmarchal @fredroy @olivier-roussel
[all] Rename DefaultContactManager into CollisionResponse (sofa-frame…
b02c005 
…work#3891)

* Rename file DefaultContactManager into CollisionResponse

* Update Compat layer with the new class name

* udpate CMakeLists with new class name

* udpate CMakeLists with new class name keeping compat file

* update class name in the code

* propagate the change of C++ class name

* add compatibility file

* update all scenes with new component name CollisionResponse

* Apply suggestions from code review

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [LinearAlgebra] Change access specifier for the method set (sofa-framework#3834)

* Change access specifier for the method set. Needed in SofaCUDASolver plugin. Regardless the resons of this modification, The fact that this method is at the begining of the class declaration without any access specifier befoe is not usual

* Changed attributs/methods orders and remove repeated identifiers

---------

Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>

* [Spring] Remove unused dependencies (sofa-framework#3848)

* [StateContainer] Fix bug in dynamic data registration (sofa-framework#3783)

* [Sofa.Type] Replace the use of Matrix::s_identity (deprecated) by Matrix::Identity()

* [Sofa.Component.StateContainer] Fix missing registration of dynamic properties.


Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>

* [SceneChecking] Add check when setting contactStiffness uselessly (sofa-framework#3843)

* [SceneChecking] Add check when setting contactStiffness uselessly

* Separate contribution in a separate function

* [test] Fix unit test on RestShapeSpringsForceField (sofa-framework#3864)

* [plugins] Remove fetching of SofaPython (sofa-framework#3855)

* [Constraint.Lagrangian] Activate the export of lambda forces by default (sofa-framework#3857)

* [all] Change variable name supportOnlySymmetricMatrix in MParams  (sofa-framework#3861)

* [all] Change variable name supportOnlySymmetricMatrix in MechanicalParams

* propagate the supportOnlySymmetricMatrix API change

* Add deprecation

---------

Co-authored-by: Alex Bilger <alexbilger0@gmail.com>

* [Constraint.Lagrangian] Add messages when no compliance is given (sofa-framework#3858)

* [Constraint.Lagrangian] Add messages when no compliance is given

* minor esthetic msg update

* [all] include base class inl file (sofa-framework#3865)

* [test] Fix failing unit test (sofa-framework#3876)

Introduced in sofa-framework#3858

* [LinearAlgebra] Pull Insimo's CompressedRowSparseMatrix into the main branch (sofa-framework#3515)

* WIP

* CRS: integration from issofa

* rename all bloc to blocks

* use MessagingAPI (and rename again bloc to block

* add matrix_bloc_traits for Vec

* fix compilation with CRS debug macros

* revert some useless changes

* add insimo as contributors

* fix compilation test by bringing back operators and MatrixExpr

* fix stuff about add()

* add compress() after clearing (fix some tests?)

* fix debug compilation

* clarify add() functions (indices, etc)

* apply insimo code and clean ups

* rename CRS for CRSGeneric

* apply insimo code for CRSMechanical (previous CRS) and clean ups

* add transitional header

* fix confusion about mul()

* bring back some changes from original version

* bring back BM:add with mat3 and various fixes

* fix gcc

* remove conversion warnings

* fix tests and stuff

* Use struct instead of class (+public)

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* remove diag/debug/trace api in crs

* remove now-useless CRS trace logger utility header

* remove some override warnings

* remove more warnings

* fix rebase

* refresh scene with MechanicalMatrixMapper

* change traits to try to match previous behavior

* fix missing include

* add explicit instanction for float type

* apply suggestion by using directly the correct type

* remove the touch trait

---------

Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>

* [SofaCUDA] Deprecated CudaTexture.h (sofa-framework#3869)

It is not used, and its content is deprecated in Cuda 12

* [examples] Fix unstable scene constantMomentum.scn (sofa-framework#3886)

* [SofaCUDA] Replace deprecated vector types (sofa-framework#3902)

* [Config] Fix cross-compilation for embedded external libs (sofa-framework#3870)

[Config] Fix cross-compilation by turning off cmake_find_root_path for embedded external libs

* [all] Deprecate unused verbose data (sofa-framework#3871)

* [all] Deprecate unused verbose data

* Add warning in parse method

* [contact] Add missing call to super init (sofa-framework#3884)

* [SolidMechanics] Use accessors & make geometrical data required in BFF (sofa-framework#3887)

[SolidMechanics] Use accessors & make geometrical data required in BeamFEMFF

* [SofaCUDA] No longer use deprecated texture references in HexaTLED (sofa-framework#3868)

* [SofaCUDA] Remove non-working scene

* Introduce example

* nodesPerElement

* DhC0, DhC1 and DhC2

* detJ

* Disp

* hourglass control

* preferredDirection

* Di1, Di2, Dv1, Dv2

* force coordinates

* Fi

* x, x0

* Ignore added scene on CI

---------

Signed-off-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alxbilger@users.noreply.github.com>
Co-authored-by: Paul Baksic <30337881+bakpaul@users.noreply.github.com>
Co-authored-by: Paul Baksic <paul.baksic@unistra.fr>
Co-authored-by: Damien Marchal <damien.marchal@univ-lille1.fr>
Co-authored-by: Alex Bilger <alexbilger0@gmail.com>
Co-authored-by: Frederick Roy <fredroy@users.noreply.github.com>
Co-authored-by: Olivier Roussel <olivier.roussel@inria.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alxbilger alxbilger alxbilger approved these changes

Assignees

No one assigned

Labels

pr: enhancement About a possible enhancement pr: status ready Approved a pull-request, ready to be squashed

Projects

None yet

Milestone

v23.12

Development

Successfully merging this pull request may close these issues.

5 participants

@bakpaul @fredroy @alxbilger @damienmarchal @hugtalbot