Add cgls-shift algorithm
#853
Conversation
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
Co-authored-by: Dominique <[email protected]>
* Update pipeline.yml
|
| expected_cgls_lanczos_shift_bytes = storage_cgls_lanczos_shift_bytes(m, k, nshifts) | ||
| (x, stats) = cgls_lanczos_shift(Ao, b, shifts) # warmup | ||
| actual_cgls_lanczos_shift_bytes = @allocated cgls_lanczos_shift(Ao, b, shifts) | ||
| @test expected_cgls_lanczos_shift_bytes ≤ actual_cgls_lanczos_shift_bytes ≤ 1.03 * expected_cgls_lanczos_shift_bytes |
There was a problem hiding this comment.
@amontoison I replaced 1.02 used here by 1.03 and it sort of "fix" the tests with Julia 1.6. What do you think?
There was a problem hiding this comment.
Try a bigger linear system ;)
|
|
||
| #### Input arguments | ||
|
|
||
| * `A`: a linear operator that models a Hermitian matrix of dimension n; |
There was a problem hiding this comment.
A is a rectangular matrix not a square Hermitian one.
|
|
||
| #### Keyword arguments | ||
|
|
||
| * `M`: linear operator that models a Hermitian positive-definite matrix of size `n` used for centered preconditioning; |
There was a problem hiding this comment.
Many keyword arguments are missing in the signature at the top of the docstring (atol, rtol, ldiv, M, ...).
| * `check_curvature`: if `true`, check that the curvature of the quadratic along the search direction is positive, and abort if not, unless `linesearch` is also `true`; | ||
| * `atol`: absolute stopping tolerance based on the residual norm; | ||
| * `rtol`: relative stopping tolerance based on the residual norm; | ||
| * `itmax`: the maximum number of iterations. If `itmax=0`, the default number of iterations is set to `2n`; |
There was a problem hiding this comment.
If A is rectangular, it's 2 × dimension(A'*A) in practice. We should verify that it's 2n and not 2m.
|
|
||
| def_kwargs_cgls_lanczos_shift = (:(; M = I ), | ||
| :(; ldiv::Bool = false ), | ||
| :(; check_curvature::Bool = false), |
There was a problem hiding this comment.
Not needed because A'A + \lambda I is always SPD due to the structure. In CG-LANCZOS-SHIFT, it's just A + \lambda I.
| @@ -42,6 +42,10 @@ | |||
| @test norm((A - I) * x[1] - b) ≤ Κ * (atol + norm(b) * rtol) | |||
| @test norm((A + I) * x[2] - b) ≤ Κ * (atol + norm(b) * rtol) | |||
| @test eltype(x) == Vector{FC} | |||
| elseif fn == :cgls_lanczos_shift | |||
| @test norm(A' * (b - A * x[1]) + x[1]) ≤ Κ * (atol + norm(b) * rtol) | |||
There was a problem hiding this comment.
| @test norm(A' * (b - A * x[1]) + x[1]) ≤ Κ * (atol + norm(b) * rtol) | |
| @test norm(A' * (b - A * x[1]) + x[1]) ≤ Κ * (atol + norm(A' * b) * rtol) |
| @@ -42,6 +42,10 @@ | |||
| @test norm((A - I) * x[1] - b) ≤ Κ * (atol + norm(b) * rtol) | |||
| @test norm((A + I) * x[2] - b) ≤ Κ * (atol + norm(b) * rtol) | |||
| @test eltype(x) == Vector{FC} | |||
| elseif fn == :cgls_lanczos_shift | |||
| @test norm(A' * (b - A * x[1]) + x[1]) ≤ Κ * (atol + norm(b) * rtol) | |||
| @test norm(A' * (b - A * x[2]) - x[2]) ≤ Κ * (atol + norm(b) * rtol) | |||
There was a problem hiding this comment.
| @test norm(A' * (b - A * x[2]) - x[2]) ≤ Κ * (atol + norm(b) * rtol) | |
| @test norm(A' * (b - A * x[2]) - x[2]) ≤ Κ * (atol + norm(A' * b) * rtol) |
| @@ -4,8 +4,8 @@ function test_solvers(FC) | |||
| m = div(n, 2) | |||
| Au = A[1:m,:] # Dimension m x n | |||
| Ao = A[:,1:m] # Dimension n x m | |||
| b = Ao * ones(FC, m) # Dimension n | |||
| c = Au * ones(FC, n) # Dimension m | |||
| b = Ao * ones(FC, m) # Dimension m | |||
There was a problem hiding this comment.
Why did you modify the dimension?
There was a problem hiding this comment.
Good work @tmigot!
Can you explain what are the differences in terms of implementation between cgls-lanczos-shift and cg-lanczos-shift when you provide A'A as input?
It seems that you are not doing the products with A and A' so I suppose that it's a trick of the implementation.
We could add a comment about it in the docstring.
I'm curious to also compare the convergence history of cgls-shift and cg-shift on an ill-conditioned LS problem.
|
@tmigot Any update? |
|
Hey @amontoison ! I didn't find the time to figure out the changes to add the preconditioner, so no real update. |
cgls_lanczos_shift.jlin the foldersrcinclude("cgls_lanczos_shift.jl")insrc/Krylov.jltest_cgls_lanczos_shift.jlin the foldertestinclude("test_cgls_lanczos_shift.jl")intest/runtests.jlCglsLanczosShiftSolverstructure insrc/krylov_solvers.jlexport CglsLanczosShiftSolverinsrc/krylov_solvers.jlKRYLOV_SOLVERSinsrc/krylov_solvers.jlfor (KS, fun, nsol, ...)at the end ofsrc/krylov_solvers.jlCglsLanczosShiftSolverindocs/src/api.md(section Solver Types)@docs cgls_lanczos_shift cgls_lanczos_shift!in the most relevant markdown file ofdocs/src/solversCGLS_LANCZOS_SHIFTintest/test_allocations.jl:cgls_lanczos_shiftintest/test_mp.jlCglsLanczosShiftSolverintest/test_solvers.jlAdd a test for the warm-start feature ofCGLS_LANCZOS_SHIFTintest/test_warm_start.jlCGLS_LANCZOS_SHIFTintest/test_verbose.jlCGLS_LANCZOS_SHIFTindocs/src/preconditioners.mdCGLS_LANCZOS_SHIFTindocs/src/processes.mdCGLS_LANCZOS_SHIFTindocs/src/factorization-free.md(not needed for methods with multiple shifts)CGLS_LANCZOS_SHIFTindocs/src/storage.mdcgls_lanczos_shift.mdin the folderdocs/src/examplesand updatedocs/make.jlCglsLanczosShiftSolverin a note ofdocs/src/inplace.mdif the solver requires the argumentmemoryornshiftssolve!andcgls_lanczos_shift!insrc/krylov_solve.jl