Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
ma-sadeghi opened this issue Oct 21, 2023 · 8 comments
Closed

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

ma-sadeghi opened this issue Oct 21, 2023 · 8 comments

Comments

@ma-sadeghi
Copy link

No description provided.

@amontoison
Copy link
Member

b can be a vector of complex numbers.

@ma-sadeghi
Copy link
Author

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
Copy link
Member

amontoison commented Oct 21, 2023

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
Copy link
Author

Thanks! Yes, but only when using GPU.

@amontoison
Copy link
Member

amontoison commented Oct 23, 2023

@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
Copy link
Author

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

@amontoison
Copy link
Member

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
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants