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

Absolute and relative tolerance for linear solvers #280

Merged
merged 18 commits into from
Dec 13, 2020

Conversation

ranocha
Copy link
Member

@ranocha ranocha commented Dec 7, 2020

This PR adds keyword arguments abstol and reltol to iterative solvers for linear system and deprecates the former keyword tol (which is now reltol).

  • BiCGStab(l)
  • CG
  • Chebyshev
  • GMRES
  • IDR(s)
  • MINRES
  • QMR
  • LSMR doesn't seem to fit to this common framework
  • LSQR doesn't seem to fit to this common framework

Since this is my first contribution to IterativeSolvers.jl, I would like to get some feedback on this work to improve this PR. Thanks!

If this approach is acceptable, I will continue to work on the other iterative solvers and adapt them accordingly.

By the way, I noticed that CG doesn't respect the stopping criterion described in the docs, see also #244. I think it would be good to fix this (as described in my comment in cg.jl).

Closes #207 by adding the keyword argument abstol.

Closes #244 by fixing the convergence criterion for CG (reltol should be relative to the initial residual instead of the RHS b).

src/cg.jl Outdated Show resolved Hide resolved
@ranocha
Copy link
Member Author

ranocha commented Dec 8, 2020

I made all the changes to the iterative solvers for "simple" linear problems. Since the solvers for least-squares problems use a different setup of tolerances, I didn't adapt them. It would be nice to have another round of reviews to improve this PR 🙂

@ranocha ranocha changed the title WIP: Absolute and relative tolerance for linear solvers Absolute and relative tolerance for linear solvers Dec 8, 2020
test/idrs.jl Outdated Show resolved Hide resolved
@ranocha ranocha requested review from mschauer and haampie December 9, 2020 05:36
test/idrs.jl Outdated Show resolved Hide resolved
@ranocha ranocha requested a review from mschauer December 9, 2020 10:02
@mschauer
Copy link
Member

mschauer commented Dec 9, 2020

Can one express the logic/stopping criteria with reltol and abstol using isapprox from base?

@ranocha
Copy link
Member Author

ranocha commented Dec 9, 2020

Can one express the logic/stopping criteria with reltol and abstol using isapprox from base?

isapprox(x, y) returns true if

norm(x-y) <= max(atol, rtol*max(norm(x), norm(y)))

Our stopping criterion is

norm(residual) <= max(abstol, reltol * initial_residual_norm)

Hence, I don't see a possibility to do that.

@mschauer
Copy link
Member

mschauer commented Dec 9, 2020

I see, thanks.

@ranocha
Copy link
Member Author

ranocha commented Dec 11, 2020

@haampie Can we merge this? The decreased coverage is only caused by the new depwarns.

@haampie
Copy link
Member

haampie commented Dec 11, 2020

Hi @ranocha , I haven't had the chance to review yet. Tomorrow I can do that

@haampie haampie merged commit c0a9e44 into JuliaLinearAlgebra:master Dec 13, 2020
@haampie
Copy link
Member

haampie commented Dec 13, 2020

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add abs_tol as termination criterion
3 participants