JuliaSmoothOptimizers / Krylov.jl

A Julia Basket of Hand-Picked Krylov Methods
Other
338 stars 51 forks source link

Automatically promote `atol` and `rtol` to `eltype(b)` #828

Closed ma-sadeghi closed 9 months ago

amontoison commented 10 months ago

b can be a vector of complex numbers.

ma-sadeghi commented 10 months ago

Right, but I meant more like CPU vs. GPU. Currently, it's a bit annoying that the user needs to adjust rtol and atol depending on the target hardware. So maybe: eltype(b[1].re) would work?

amontoison commented 10 months ago

The correct solution is real(eltype(b)) but I need to update many files for that. Are you solving linear systems in single precision?

ma-sadeghi commented 10 months ago

Thanks! Yes, but only when using GPU.

amontoison commented 10 months ago

@ma-sadeghi I checked a little bit tonight what are the consequences if I update the type of atol and rtol. If I replace T by AbstractFloat, Julia precompiles a new method (see here) everytime that you call the Krylov method with a different type for atol or rtol.

cg(A, b, atol=1e-8, rtol=1e-8)
cg(A, b, atol=Float32(1e-8), rtol=1e-8)
cg(A, b, atol=1e-8, rtol=Float32(1e-8))
cg(A, b, atol=Float32(1e-8), rtol=Float32(1e-8))

I would like to avoid that because I plan to use PrecompileTools.jl in the future (#727) to precompile the Krylov methods.

ma-sadeghi commented 10 months ago

Thanks for looking into it. Can't we do the promotion inside the function body? (not at the args level)

amontoison commented 10 months ago

We can but Julia will precompile a new version of gmres, cg, etc... because the type of the argument for atol or rtol is new. It's what I tried to explain with my previous example. Each call in my previous comment triggers a compilation.

ma-sadeghi commented 10 months ago

I see, thanks. I guess it's okay given that Krylov.jl is a "base" package, not a wrapper, so explicit is better than implicit. LinearSolve.jl, which is just a wrapper, has been patched up (SciML/LinearSolve.jl/pull/397), so it's all good.