Skip to content

Commit acce770

Browse files
authored
IterativeSolvers: accept LinearSolve's maxiters spelling on the algorithm (#1196)
1 parent e623b1c commit acce770

2 files changed

Lines changed: 53 additions & 8 deletions

File tree

ext/LinearSolveIterativeSolversExt.jl

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ end
5959

6060
LinearSolve._isidentity_struct(::IterativeSolvers.Identity) = true
6161

62+
# Accept LinearSolve's `maxiters` spelling on the algorithm and hand
63+
# IterativeSolvers the `maxiter` it expects. An explicit `maxiter` wins if both
64+
# are given, and the NamedTuple is rebuilt rather than mutated so the result
65+
# stays inferrable.
66+
function _rename_maxiters(kwargs)
67+
haskey(kwargs, :maxiters) || return NamedTuple(kwargs)
68+
nt = NamedTuple(kwargs)
69+
maxiters = nt.maxiters
70+
rest = Base.structdiff(nt, NamedTuple{(:maxiters,)})
71+
return haskey(rest, :maxiter) ? rest : merge(rest, (; maxiter = maxiters))
72+
end
73+
6274
function LinearSolve.init_cacheval(
6375
alg::IterativeSolversJL, A, b, u, Pl, Pr, maxiters::Int,
6476
abstol,
@@ -77,9 +89,18 @@ function LinearSolve.init_cacheval(
7789
restart = (alg.gmres_restart == 0) ? min(20, size(A, 1)) : alg.gmres_restart
7890
s = get(alg.kwargs, :idrs_s, 4) # shadow space
7991

92+
# LinearSolve spells the iteration cap `maxiters`, IterativeSolvers spells it
93+
# `maxiter`. Passing the LinearSolve spelling on the algorithm, as in
94+
# `IterativeSolversJL_CG(maxiters = 100)`, used to forward an unknown keyword
95+
# and fail with a MethodError (SciML/LinearSolve.jl#175), so accept it as an
96+
# alias here. Everything below reads the cap from `maxiters_eff` so the
97+
# algorithm-level value also reaches the solvers that take it positionally.
98+
alg_kwargs = _rename_maxiters(alg.kwargs)
99+
maxiters_eff = get(alg_kwargs, :maxiter, maxiters)
100+
80101
kwargs = (
81-
abstol = abstol, reltol = reltol, maxiter = maxiters,
82-
alg.kwargs...,
102+
abstol = abstol, reltol = reltol, maxiter = maxiters_eff,
103+
alg_kwargs...,
83104
)
84105

85106
iterable = if alg.generate_iterator === IterativeSolvers.cg_iterator!
@@ -119,26 +140,29 @@ function LinearSolve.init_cacheval(
119140
return kwargs
120141
end
121142
IterativeSolvers.idrs_iterable!(
122-
history, u, A, b, s, Pl, abstol, reltol, maxiters;
123-
filter_kwargs(; alg.kwargs...)...
143+
history, u, A, b, s, Pl, abstol, reltol, maxiters_eff;
144+
filter_kwargs(; alg_kwargs...)...
124145
)
125146
elseif alg.generate_iterator === IterativeSolvers.bicgstabl_iterator!
126147
!!LinearSolve._isidentity_struct(Pr) &&
127148
@SciMLMessage(
128149
"$(alg.generate_iterator) doesn't support right preconditioning",
129150
verbosity, :no_right_preconditioning
130151
)
152+
# `bicgstabl_iterator!` caps work through `max_mv_products`, set just
153+
# above, and has no `maxiter` keyword at all, so the normalized cap has
154+
# to come out again here.
131155
alg.generate_iterator(
132156
u, A, b, alg.args...; Pl = Pl,
133157
abstol = abstol, reltol = reltol,
134-
max_mv_products = maxiters * 2,
135-
alg.kwargs...
158+
max_mv_products = maxiters_eff * 2,
159+
Base.structdiff(alg_kwargs, NamedTuple{(:maxiter,)})...
136160
)
137161
else # minres, qmr
138162
alg.generate_iterator(
139163
u, A, b, alg.args...;
140-
abstol = abstol, reltol = reltol, maxiter = maxiters,
141-
alg.kwargs...
164+
abstol = abstol, reltol = reltol, maxiter = maxiters_eff,
165+
alg_kwargs...
142166
)
143167
end
144168
return iterable

test/Core/basictests.jl

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -766,6 +766,27 @@ end
766766
@test sol.u u5 rtol = 1.0e-6
767767
end
768768
end
769+
770+
@testset "maxiters on the algorithm (#175)" begin
771+
# LinearSolve spells it `maxiters`, IterativeSolvers `maxiter`.
772+
# Passing the LinearSolve spelling on the algorithm used to
773+
# forward an unknown keyword and raise a MethodError.
774+
for f in (
775+
IterativeSolversJL_CG, IterativeSolversJL_GMRES,
776+
IterativeSolversJL_IDRS, IterativeSolversJL_MINRES,
777+
IterativeSolversJL_BICGSTAB,
778+
)
779+
@test solve(LinearProblem(A5, b5), f(maxiters = 200)).u u5 rtol = 1.0e-6
780+
end
781+
782+
# The algorithm-level value caps the iteration count, and an
783+
# explicit `maxiter` still wins if both are given.
784+
slow = LinearProblem(Symmetric(Matrix(A5) + 0.5I), b5)
785+
for m in (2, 5)
786+
@test solve(slow, IterativeSolversJL_CG(maxiters = m)).iters <= m
787+
end
788+
@test solve(slow, IterativeSolversJL_CG(maxiter = 3, maxiters = 50)).iters == 3
789+
end
769790
end
770791
end
771792

0 commit comments

Comments
 (0)