JuliaLinearAlgebra / IterativeSolvers.jl

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

CG's stopping criteria #244

Closed nrontsis closed 3 years ago

nrontsis commented 5 years ago

The problem

CG does not respect the stopping criterion of the documentation:

|r_k| / |r_0| ≤ tol

as it can be observed from the following minimal example:

using IterativeSolvers

n = 3
A = Matrix(I, n, n)
b = randn(n)

x = b + 1e-5*randn(n) # Initialize x close to the solution b
tol = 1e-3
initial_residual = norm(A*x - b)

output, history = cg!(x, A, b, tol=tol, initially_zero=false, log=true, verbose=true)
@assert norm(A*x - b)/initial_residual <= tol

It appears that cg stops when |r_k| / |b| ≤ tol. However, I am unsure if this actually holds, as the CGStateVariables are filled with vectors generated with the similar() command which can be problematic, as it does not initialise the memory of the vectors.

Suggested solution

Change the stopping criterion to match the one of the documentation and replace similar() with zero().

I suppose that another possibility would be to change the documentation? Let me know what you think!

codecov-io commented 5 years ago

Codecov Report

Merging #244 into master will decrease coverage by 0.18%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #244      +/-   ##
==========================================
- Coverage   90.52%   90.34%   -0.19%     
==========================================
  Files          17       17              
  Lines        1077     1077              
==========================================
- Hits          975      973       -2     
- Misses        102      104       +2
Impacted Files Coverage Δ
src/cg.jl 90.69% <100%> (-4.66%) :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 17ef261...263dce7. Read the comment docs.

chriscoey commented 5 years ago

bump!

ranocha commented 3 years ago

Bump. That's a good observation! See also #280.