Skip to content

[SofaImplicitOdeSolver] Rewrite of the static Newton-Raphson ODE solver#2050

Merged
jnbrunet merged 6 commits into
sofa-framework:masterfrom
jnbrunet:improve_static_solver
May 26, 2021
Merged

[SofaImplicitOdeSolver] Rewrite of the static Newton-Raphson ODE solver#2050
jnbrunet merged 6 commits into
sofa-framework:masterfrom
jnbrunet:improve_static_solver

Conversation

@jnbrunet

@jnbrunet jnbrunet commented Apr 28, 2021

Copy link
Copy Markdown
Contributor

This commit rewrites the static ODE solver, implements two additional convergence criteria, improves the documentation of every step found in the "solve" method, and adds useful doxygen documentation of the Newton algorithms executed by the component.

Up until this commit, the only convergence criteria of the static ODE solver were absolute force residual norms, and absolute (displacement) increment norm. Using absolute values for convergence criteria means that the value set by the user is dependent on the simulation scale. For example, setting an absolute displacement norm of 1e-5 might seem enough for a liver mesh set in millimetres, but is definitely not adequate for the same liver using this time a mesh scaled in metres. This is a source of user error, and is probably why many FEM software will use relative convergence criteria.

The two additional criteria are:

  1. The relative residual norm |R|/|R0| where |R| is the norm of the force vector after the last Newton iteration, and |R0| is the norm of the force vector after the first Newton iteration.
  2. The relative displacement norm |dx|/|U| where |dx| is the norm of the latest solution vector, and |U| is the norm of the sum over all previous solution vectors of the current time step.

To remove any confusion between absolute and relative criteria, the convergence data field "correction_tolerance_threshold" and "residual_tolerance_threshold" were renamed for "absolute_correction_tolerance_threshold" and "absolute_residual_tolerance_threshold", respectively. Note that to keep a compatibility layer with older scene files, these old data field names are still available, but a deprecated warning is shown to the user, inviting him to update its scene.

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

EDIT: Fix #224


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

@jnbrunet jnbrunet added pr: enhancement About a possible enhancement pr: status to review To notify reviewers to review this pull-request labels Apr 28, 2021
@jnbrunet
jnbrunet requested a review from hugtalbot April 28, 2021 11:10
@jnbrunet jnbrunet self-assigned this Apr 28, 2021
@jnbrunet
jnbrunet force-pushed the improve_static_solver branch from 4b08366 to 9aa2e09 Compare April 28, 2021 11:35
@jnbrunet

Copy link
Copy Markdown
Contributor Author

Setting back to WIP after a quick review

  1. absolute and relative residual are inverted in the convergence checks
  2. Missing the divergence test when the residual is growing
  3. I will add unit tests for relative norms

@jnbrunet jnbrunet 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 28, 2021
This commit rewrites the static ODE solver, implements two additional convergence criteria, improves the documentation of every step found in the "solve" method, and adds useful doxygen documentation of the Newton algorithms executed by the component.

Up until this commit, the only convergence criteria of the static ODE solver were absolute force residual norms, and absolute (displacement) increment norm. Using absolute values for convergence criteria means that the value set by the user is dependent on the simulation scale. For example, setting an absolute displacement norm of 1e-5 might seem enough for a liver mesh set in millimetres, but is definitely not adequate for the same liver using this time a mesh scaled in metres. This is a source of user error, and is probably why many FEM software will use relative convergence criteria.

The two additional criteria are:

The relative residual norm |R|/|R0| where |R| is the norm of the force vector after the last Newton iteration, and |R0| is the norm of the force vector after the first Newton iteration.
The relative displacement norm |dx|/|U| where |dx| is the norm of the latest solution vector, and |U| is the norm of the sum over all previous solution vectors of the current time step.
To remove any confusion between absolute and relative criteria, the convergence data field "correction_tolerance_threshold" and "residual_tolerance_threshold" were renamed for "absolute_correction_tolerance_threshold" and "absolute_residual_tolerance_threshold", respectively. Note that to keep a compatibility layer with older scene files, these old data field names are still available, but a deprecated warning is shown to the user, inviting him to update its scene.
@jnbrunet
jnbrunet force-pushed the improve_static_solver branch from 9aa2e09 to 3ba0e37 Compare April 28, 2021 13:48
@jnbrunet jnbrunet 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 28, 2021
@jnbrunet

Copy link
Copy Markdown
Contributor Author

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

@alxbilger alxbilger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Excellent PR. Thanks a lot for all the details you wrote in the comment. It helps a lot to understand all the steps of a solver, and how to execute them in Sofa.

I am really concerned about this operator. It hides a lot of steps, and it is easy to miss it. So thanks again to highlight it in the comments.

Cosmetic: use Allman style (brace associated with a control statement on the next line)

@jnbrunet

jnbrunet commented May 5, 2021

Copy link
Copy Markdown
Contributor Author

Thanks a lot for the review @alxbilger

I am really concerned about this operator. It hides a lot of steps, and it is easy to miss it.

Totally agree with you here.

# Conflicts:
#	SofaKernel/modules/SofaImplicitOdeSolver/src/SofaImplicitOdeSolver/StaticSolver.cpp
@guparan guparan changed the title [SofaKernel] Rewrite of the static Newton-Raphson ODE solver [SofaImplicitOdeSolver] Rewrite of the static Newton-Raphson ODE solver May 5, 2021
@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 May 5, 2021
@jnbrunet jnbrunet 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, 2021
@jnbrunet

jnbrunet commented May 6, 2021

Copy link
Copy Markdown
Contributor Author

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

@fredroy

fredroy commented May 12, 2021

Copy link
Copy Markdown
Contributor

Seems OK, could you just rebase/merge with master to remove the failed CUDA/whatever scenes? Just to be in the clear.

@jnbrunet

Copy link
Copy Markdown
Contributor Author

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

@sofabot

sofabot commented May 21, 2021

Copy link
Copy Markdown
Collaborator

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

To unlock the merge button, you must

@sofabot

sofabot commented May 21, 2021

Copy link
Copy Markdown
Collaborator

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

To unlock the merge button, you must

@sofabot

sofabot commented May 21, 2021

Copy link
Copy Markdown
Collaborator

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

To unlock the merge button, you must

@sofabot

sofabot commented May 21, 2021

Copy link
Copy Markdown
Collaborator

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

To unlock the merge button, you must

@sofabot

sofabot commented May 25, 2021

Copy link
Copy Markdown
Collaborator

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

To unlock the merge button, you must

@jnbrunet

Copy link
Copy Markdown
Contributor Author

[ci-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 26, 2021
@guparan

guparan commented May 26, 2021

Copy link
Copy Markdown
Contributor

[ci-build]

@sofabot

sofabot commented May 26, 2021

Copy link
Copy Markdown
Collaborator

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

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

@jnbrunet
jnbrunet merged commit 97760a9 into sofa-framework:master May 26, 2021
@jnbrunet
jnbrunet deleted the improve_static_solver branch May 26, 2021 11:09
@guparan guparan added this to the v21.06 milestone Jun 28, 2021
damienmarchal pushed a commit to CRIStAL-PADR/sofa that referenced this pull request Oct 27, 2025
…the Static ODE solve (sofa-framework#147)

See SOFA PR sofa-framework#2050

Signed-off-by: Jean-Nicolas Brunet <jnbrunet2000@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Share in SOFA the StaticSolver

5 participants