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

[LinearSolver] Remove CSparse-based linear solvers #4258

Closed alxbilger closed 5 months ago

alxbilger commented 6 months ago

They are moved in an external plugin: https://github.com/sofa-framework/CSparseSolvers

Two components are impacted: SparseLUSolver and SparseCholeskySolver.

The plugin CSparseSolvers is now fetchable

⚠️ SparseLUSolver has an equivalent using Eigen in SOFA (without the need of a plugin): EigenSparseLU, and SparseCholeskySolver equivalent is EigenSimplicialLLT.

[ci-depends-on https://github.com/sofa-framework/SofaPython3/pull/378] [ci-depends-on https://github.com/sofa-framework/BeamAdapter/pull/119]

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

fredroy commented 6 months ago

@alxbilger you did not keep the (git) history of the files when moving to the new plugin.

alxbilger commented 6 months ago

@alxbilger you did not keep the (git) history of the files when moving to the new plugin.

It's such a pain. I suspect you don't like me much to request such a task. In this branch https://github.com/alxbilger/CSparseSolvers/tree/move_csparse_files, I have the same plugin but with all the SOFA history. I guess it's not really what we want neither. Do you know how to proceed?

fredroy commented 6 months ago

@alxbilger you did not keep the (git) history of the files when moving to the new plugin.

It's such a pain. I suspect you don't like me much to request such a task.

🥰

In this branch https://github.com/alxbilger/CSparseSolvers/tree/move_csparse_files, I have the same plugin but with all the SOFA history. I guess it's not really what we want neither. Do you know how to proceed?

I would proceed like this:

Credit to @hugtalbot https://github.com/sofa-framework/sofa/issues/1442

alxbilger commented 6 months ago

@fredroy the history is now preserved

alxbilger commented 6 months ago

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

sofabot commented 6 months ago

[ci-depends-on] detected during build #5.

To unlock the merge button, you must

fredroy commented 6 months ago

@fredroy the history is not preserved

It does now no ? I can see commits down to Commits on Mar 10, 2009 ? (a9bd817a4ec46a06e07b9f755660c58d48cf9d17)

alxbilger commented 6 months ago

@fredroy the history is not preserved

It does now no ? I can see commits down to Commits on Mar 10, 2009 ? (a9bd817)

Yes, it is preserved

fredroy commented 6 months ago

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

sofabot commented 6 months ago

[ci-depends-on] detected during build #6.

To unlock the merge button, you must

sofabot commented 6 months ago

[ci-depends-on] detected during build #7.

To unlock the merge button, you must

fredroy commented 6 months ago

@alxbilger it seems that some scenes which use EigenSimplicialLLT instead of a CSparse-based linear solver are crashing, maybe because of the default template CSRMat3x3

bakpaul commented 6 months ago

Why is the minimum cmake version of CSparsePlugin 3.24 -> ci won't work if it stays like that. I've configured it with 3.22 (shipped version of Ubuntu 22.04) and it works fine. (I know not exactly in this PR, but this is the pr that propose the CSparePlugin creation)

alxbilger commented 6 months ago

@bakpaul The 3.24 version of CMake was in anticipation of @olivier-roussel's work on fetching mechanism. But since the 3.24 version is no longer required, it could be the same than SOFA.

hugtalbot commented 6 months ago

TODO: save the previous scenes in the new plugin

sofabot commented 5 months ago

[ci-depends-on] detected during build #8.

To unlock the merge button, you must

alxbilger commented 5 months ago

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

alxbilger commented 5 months ago

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

sofabot commented 5 months ago

[ci-depends-on] detected during build #10.

To unlock the merge button, you must

alxbilger commented 5 months ago

@bakpaul This PR does not take the latest commits in sofa-framework/ci. Hence, it does not fetch and compile the plugin CSparseSolvers. Here is what I see in the logs:

--------------- Clone CI scripts ---------------
pwd = /builds/workspace/sofa-framework/PR-4258/ubuntu_gcc_options_release
Cloning into 'ci'...
640400517e592bfa826a46dea29b6ddd512abf02 Merge pull request #21 from hugtalbot/202308_remove_meshsteploader_from_list
CI scripts are available in /builds/workspace/sofa-framework/PR-4258/ubuntu_gcc_options_release/ci/scripts

It's not the latest commit in the master branch. Can you investigate why?

bakpaul commented 5 months ago

My bad, I modified the CI to checkout before your changes because I though that I merged too early, but I finally realized that it was useless. So back to normal !

alxbilger commented 5 months ago

ok thanks ;)

alxbilger commented 5 months ago

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

sofabot commented 5 months ago

[ci-depends-on] detected during build #11.

To unlock the merge button, you must

sofabot commented 5 months ago

[ci-depends-on] detected during build #12.

To unlock the merge button, you must

fredroy commented 5 months ago

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

sofabot commented 5 months ago

[ci-depends-on] detected during build #13.

All dependencies are merged/closed and all ExternalProject pointers are up-to-date. Congrats! :+1: