search

JuliaLinearAlgebra / IterativeSolvers.jl

Iterative algorithms for solving linear systems, eigensystems, and singular value problems
MIT License
399 stars 105 forks source link

Return CGResult from cg and allow absolute tol #238

Open mohamed82008 opened 5 years ago

mohamed82008 commented 5 years ago

This fixes #236. I also allowed passing absolute tolerance as opposed to just a relative one. Since it makes sense for the residual to be <= the tolerance, the absolute tolerance is the one that should be returned in the CGResult. And it would be weird if I pass tol = .. to cg and then I see a different tolerance printed in the result.

codecov-io commented 5 years ago

Codecov Report

Merging #238 into master will decrease coverage by 0.48%. The diff coverage is 30.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
- Coverage   56.03%   55.54%   -0.49%     
==========================================
  Files          17       17              
  Lines        1740     1757      +17     
==========================================
+ Hits          975      976       +1     
- Misses        765      781      +16
Impacted Files Coverage Δ
src/cg.jl 52.5% <30.76%> (-12.58%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 91cac26...8c03b4b. Read the comment docs.

mohamed82008 commented 5 years ago

I believe this is ready for a first review.

pkofod commented 5 years ago

How do you plan to deprecate tol? I think it'd be better to add an abs_tol and rel_tol, have tol default to nothing and throw a deprecation warning if tol is actually set.

(I came here because I wanted to use abstol in gmres for a inexact newton method, so I'd love if all methods got this abs/rel_tol separation. Isn't base using abstol and reltol btw?)

mohamed82008 commented 5 years ago

I don't mind getting rid of tol. Not big on the underscore either.

pkofod commented 5 years ago

Good. Then I think introducing abstol and reltol and deprecating the tol keyword is the best way forward.

chriscoey commented 5 years ago

bump

ranocha commented 3 years ago

I'm working on introducing keyword arguments abstol and reltol in #280 with a proper deprecation of tol. Once that's settled, this approach to change the return type would be nice to consider next.

mschauer commented 2 years ago

280 is merged now