getkeops / keops

KErnel OPerationS, on CPUs and GPUs, with autodiff and without memory overflows
https://www.kernel-operations.io
MIT License
1.02k stars 65 forks source link

Use of -use_fast_math CUDA compiler option #333

Open Turakar opened 9 months ago

Turakar commented 9 months ago

https://github.com/getkeops/keops/blob/52ed22a7fbbcf4bd02dbdf5dc2b00bf79cceddf5/keopscore/keopscore/binders/nvrtc/nvrtc_jit.cpp#L68C45-L68C58

Use of -use_fast_math without proper verification of accuracy seems like a dangerous thing to do. For example, the sine/cosine functions become imprecise outside of $[-\pi, \pi]$ (reference), which might happen quickly for high-frequency periodic kernels. I think this should at least become an option, and one might consider turning it off by default.

A nicer approach would be the replacement on a per-function basis as it is recommended in the CUDA programming guide, for example via Exp_, Sin_, ... and so on.

Did you check the accuracy of fast math?

joanglaunes commented 9 months ago

Hello @Turakar , Thank you very much for making this comment, and for giving suggestions of modifications. You are completely right and we were not careful at all on this point. We will probably start by allowing the user to disable this option, then decide wether we disable it by default or not, or use two versions of functions as you suggest. We will try to address this as soon as possible.

Turakar commented 5 months ago

Hi, I see that you, @joanglaunes, started to work on this issue, thank you! Do you already have some first insights to share? I am happy to see that we now have an option to turn fast math off, to investigate the effect of it.