JuliaLinearAlgebra / IterativeSolvers.jl

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

Unifying the interface options #279

Open ranocha opened 3 years ago

ranocha commented 3 years ago

We have some applications where it would be very useful to have both absolute and relative tolerances, e.g. because we will run a lot of linear solves with good initial guesses such as in Newton methods. Currently, the linear solvers seem to support only relative tolerance (via the keyword argument tol). Would it be okay

A related issue could be to specify a divergence tolerance divtol, similar to PETSc.

Another issue concerns the maximal number of iterations or matrix-vector products. Some methods allow specifying a maxiter (e.g. GMRES) while other allow specifying a max_mv_products (e.g. BiCGStab(l)). It would be nice if all methods could use the same keyword argument for this to make it easier to switch solvers.

haampie commented 3 years ago

Hi @ranocha, I would say this is great to have. abstol and reltol are good choices, I believe there's been some people complaining atol and rtol are hard to decipher.

The depwarn is OK, especially if iterativesolvers.jl is used somewhere deep inside another algorithm. But I'm also happy to quickly create a breaking change that doesn't have the depwarn.

max_mv_products is a mistake I'd say. Let's just use maxiter throughout.

divtol is quite alright too, but do note that there's various methods that have a bumpy convergence behavior, so it'll only really make sense for methods that are known to be not-bumpy, including CG and maybe GMRES. But even then, irregular convergence can happen due to rounding errors etc, so I probably wouldn't use this divtol myself.

ranocha commented 3 years ago

Thanks for the feedback! I will add abstol and reltol in #280. We can have a look at maxiter(s) later, since that will likely be more difficult to change without breaking anything, since we should be consistent with the use of iterations in IterativeSolvers (in the convergence history etc.).

If we go for maxiter(s), I would like to use maxiters since that's in accordance with the keyword argument in the SciML ecosystem, see https://diffeq.sciml.ai/latest/basics/common_solver_opts/#Miscellaneous.

Would you propose to use maxiters as a replacement for "maximum number of matrix-vector products" and make this nomenclature consistent everywhere, i.e. should we also use iters to count the number of matrix-vector products in the convergence history or should iters still be the number of iterations?

haampie commented 3 years ago

Let's just make maxiter(s) the number of "outer" iterations of the algorithm. bicgstab(k) is special cause it has k gmres-like inner iteration per out iteration, but it's implemented as the matrix acting on a block of vectors, which is typically faster than applying it per vector, so it doesn't really make sense to count matrix-vector products in that algorithm. So instead, just count the number of outer iterations, and make the user responsible for potentially scaling that with their k param.

haampie commented 3 years ago

And yeah, maxiters is fine if that brings more consistency across packages.

ranocha commented 3 years ago

Okay, that sounds reasonable to me.