Do a final pass on the PR

JuliaSmoothOptimizers · Oct 14, 2024 · bb35757 · bb35757
1 parent 255c79e
commit bb35757
Show file tree

Hide file tree

Showing 7 changed files with 18 additions and 20 deletions.
diff --git a/docs/src/matrix_free.md b/docs/src/matrix_free.md
@@ -41,6 +41,7 @@ Some methods only require `A * v` products, whereas other ones also require `A'
 | SYMMLQ, CG-LANCZOS, MINRES, MINRES-QLP, MINARES | LSLQ, LSQR, LSMR, LNLQ, CRAIG, CRAIGMR   |
 | DIOM, FOM, DQGMRES, GMRES, FGMRES, BLOCK-GMRES  | BiLQ, QMR, BiLQR, USYMLQ, USYMQR, TriLQR |
 | CGS, BICGSTAB                                   | TriCG, TriMR                             |
+| CG-LANCZOS-SHIFT                                | CGLS-LANCZOS-SHIFT                       |
 
 !!! info
     GPMR is the only method that requires `A * v` and `B * w` products.

diff --git a/src/Krylov.jl b/src/Krylov.jl
@@ -20,7 +20,6 @@ include("car.jl")
 
 include("symmlq.jl")
 include("cg_lanczos.jl")
-include("cg_lanczos_shift.jl")
 include("minres.jl")
 include("minres_qlp.jl")
 include("minares.jl")
@@ -47,7 +46,6 @@ include("qmr.jl")
 include("bilqr.jl")
 
 include("cgls.jl")
-include("cgls_lanczos_shift.jl")
 include("crls.jl")
 
 include("cgne.jl")
@@ -61,5 +59,8 @@ include("lnlq.jl")
 include("craig.jl")
 include("craigmr.jl")
 
+include("cg_lanczos_shift.jl")
+include("cgls_lanczos_shift.jl")
+
 include("krylov_solve.jl")
 end
diff --git a/src/krylov_solvers.jl b/src/krylov_solvers.jl
@@ -1189,14 +1189,14 @@ function CglsSolver(A, b)
 end
 
 """
-Type for storing the vectors required by the in-place version of CGLS-LANCZOS-SHIFT.
+Workspace for the in-place version of CGLS-LANCZOS-SHIFT.
 
-The outer constructors
+The outer constructors:
 
     solver = CglsLanczosShiftSolver(m, n, nshifts, S)
     solver = CglsLanczosShiftSolver(A, b, nshifts)
 
-may be used in order to create these vectors.
+can be used to initialize this workspace.
 """
 mutable struct CglsLanczosShiftSolver{T,FC,S} <: KrylovSolver{T,FC,S}
   m          :: Int

diff --git a/test/runtests.jl b/test/runtests.jl
@@ -35,7 +35,6 @@ include("test_minres_qlp.jl")
 include("test_symmlq.jl")
 include("test_bilq.jl")
 include("test_cgls.jl")
-
 include("test_cgls_lanczos_shift.jl")
 include("test_crls.jl")
 include("test_cgne.jl")

diff --git a/test/test_allocations.jl b/test/test_allocations.jl
@@ -13,7 +13,7 @@
       mem = 200
 
       T = real(FC)
-      shifts  = T[1; 2; 3; 4; 5]
+      shifts = T[1; 2; 3; 4; 5]
       nshifts = 5
       nbits_FC = sizeof(FC)  # 8 bits for ComplexF32 and 16 bits for ComplexF64
       nbits_T = sizeof(T)    # 4 bits for Float32 and 8 bits for Float64
@@ -401,15 +401,14 @@
         # - 2 (n*nshifts)-matrices: x, p
         # - 5 nshifts-vectors: σ, δhat, ω, γ, rNorms
         # - 3 nshifts-bitVector: converged, indefinite, not_cv
-
         storage_cgls_lanczos_shift_bytes(m, n, nshifts) = nbits_FC * (2 * n + 3 * m + 2 * n * nshifts) + nbits_T * (5 * nshifts) + (3 * nshifts)
 
         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
 
-        solver = CglsLanczosShiftSolver(Ao, b, length(shifts))
+        solver = CglsLanczosShiftSolver(Ao, b, nshifts)
         cgls_lanczos_shift!(solver, Ao, b, shifts)  # warmup
         inplace_cgls_lanczos_shift_bytes = @allocated cgls_lanczos_shift!(solver, Ao, b, shifts)
         @test inplace_cgls_lanczos_shift_bytes == 0

diff --git a/test/test_cgls_lanczos_shift.jl b/test/test_cgls_lanczos_shift.jl
@@ -28,17 +28,15 @@ end
       end
       @test stats.status == "x = 0 is a zero-residual solution"
 
-      #=
       # Not implemented
       # Test with Jacobi (or diagonal) preconditioner
-      A, b, M = square_preconditioned(FC=FC)
-      shifts = [1.0; 2.0; 3.0; 4.0; 5.0; 6.0]
-      (x, stats) = cgls_lanczos_shift(A, b, shifts, M=M)
-      r = residuals(A, b, shifts, x)
-      resids = map(norm, r) / norm(b)
-      @test(all(resids .≤ cgls_lanczos_shift_tol))
-      @test(stats.solved)
-      =#
+      # A, b, M = square_preconditioned(FC=FC)
+      # shifts = [1.0; 2.0; 3.0; 4.0; 5.0; 6.0]
+      # (x, stats) = cgls_lanczos_shift(A, b, shifts, M=M)
+      # r = residuals(A, b, shifts, x)
+      # resids = map(norm, r) / norm(b)
+      # @test(all(resids .≤ cgls_lanczos_shift_tol))
+      # @test(stats.solved)
 
       # test callback function
       A, b = symmetric_definite(FC=FC)

diff --git a/test/test_mp.jl b/test/test_mp.jl
@@ -43,8 +43,8 @@
             @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)
+            @test norm(A' * (b - A * x[1]) + x[1]) ≤ Κ * (atol + norm(A' * b) * rtol)
+            @test norm(A' * (b - A * x[2]) - x[2]) ≤ Κ * (atol + norm(A' * b) * rtol)
             @test eltype(x) == Vector{FC}
           else
             @test norm(A * x - b) ≤ Κ * (atol + norm(b) * rtol)