You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I recently read the code of the newton solver provided by dolfinx (nls::petsc::NewtonSolver), to have more insight on how convergence criterions are implemented, and I noted that regardless of the value of convergence_criterion, the value of residual0 (which is used to compute the relative residual) is always set as the norm of the increment after the first iteration. See cpp/dolfinx/nls/NewtonSolver.cpp lines 219 to 225.
This seems wrong when the convergence criterion is set to "residual" because the residual and the increment may be of different units or order of magnitude (See cpp/dolfinx/nls/NewtonSolver.cpp line 33).
It feels more logical to set residual0 to:
The norm of the increment when convergence_criterion == "incremental".
The norm of the residual when convergence_criterion == "residual".
How to reproduce the bug
Note that this does'nt cause scripts to fail because you can adapt the value of the attributes of NewtonSolver rtol and atol to make the simulation converge, but the meaning of rtol is then unclear.
Otherwise, any script using the newton solver from dolfinx.nls.petsc set up with newton.convergence_criterion = "residual" will reproduce the error. One may for example use the code provided here by J.S. Dokken that implement the resolution of a non linear Poisson equation and change the value of solver.convergence_criterion.
The values of the relative and absolute residual can be accessed by setting the log level of dolfinx to 2 dolfinx.log.set_log_level(2).
Minimal Example (Python)
Output (Python)
Version
main branch
DOLFINx git commit
I read the C++ code on github, on the lastest commit of the main branch.
Installation
No response
Additional information
Edit: I saw that this issue was already addressed in PR #3611 just after posting, may this PR be merged in the future ?
Edit 2: I forgot to mentioned that I implemented the Newton solver in Python to change this behavior on my side. I can provide this code if this can be helpful to anyone.
The text was updated successfully, but these errors were encountered:
Uh oh!
There was an error while loading. Please reload this page.
Summarize the issue
I recently read the code of the newton solver provided by dolfinx (
nls::petsc::NewtonSolver
), to have more insight on how convergence criterions are implemented, and I noted that regardless of the value ofconvergence_criterion
, the value ofresidual0
(which is used to compute the relative residual) is always set as the norm of the increment after the first iteration. See cpp/dolfinx/nls/NewtonSolver.cpp lines 219 to 225.This seems wrong when the convergence criterion is set to
"residual"
because the residual and the increment may be of different units or order of magnitude (See cpp/dolfinx/nls/NewtonSolver.cpp line 33).It feels more logical to set
residual0
to:convergence_criterion == "incremental"
.convergence_criterion == "residual"
.How to reproduce the bug
Note that this does'nt cause scripts to fail because you can adapt the value of the attributes of NewtonSolver
rtol
andatol
to make the simulation converge, but the meaning ofrtol
is then unclear.Otherwise, any script using the newton solver from
dolfinx.nls.petsc
set up withnewton.convergence_criterion = "residual"
will reproduce the error. One may for example use the code provided here by J.S. Dokken that implement the resolution of a non linear Poisson equation and change the value ofsolver.convergence_criterion
.The values of the relative and absolute residual can be accessed by setting the log level of dolfinx to 2
dolfinx.log.set_log_level(2)
.Minimal Example (Python)
Output (Python)
Version
main branch
DOLFINx git commit
I read the C++ code on github, on the lastest commit of the main branch.
Installation
No response
Additional information
Edit: I saw that this issue was already addressed in PR #3611 just after posting, may this PR be merged in the future ?
Edit 2: I forgot to mentioned that I implemented the Newton solver in Python to change this behavior on my side. I can provide this code if this can be helpful to anyone.
The text was updated successfully, but these errors were encountered: