search

flatironinstitute / cufinufft

Nonuniform fast Fourier transforms of types 1 and 2, in 1D, 2D, and 3D, on the GPU
Other
83 stars 18 forks source link

Add checks to ensure distance is strictly less than half of kernel width #113

Closed MelodyShih closed 3 years ago

MelodyShih commented 3 years ago

This is related to #112 .

The error happens when nonuniform points lie on the fine grid. The distance between the point and the grid point are exactly kernelwidth/2 and cause exp(es_beta * (sqrt(1.0 - es_c*x*x))) evaluates to nan.

I added checks to enforce this does not happen.

ahbarnett commented 3 years ago

Yes, this was needed in FINUFFT too, when the plain exp(sqrt(..)) kernel used: https://github.com/flatironinstitute/finufft/blob/adcb1d25aa8f3b26aed1956a9dccc12bd8a78eea/src/spreadinterp.cpp#L639 With the vectorized Horner eval, there are no NaNs, so no such check was needed.

Note that your PR has an awful lot of repetition of the check. Could it be rationalized so that the kernel eval and associated check is done in one place only?

You might want to check that tweaks to rounding and wrapping in FINUFFT over the last year have been brought into cuFINUFFT. I can list commits if you need, but have a look in spreadinterp.cpp, basically. Best, Alex

MelodyShih commented 3 years ago

Hi Alex, sure, sounds better to do what you suggest. I will update with a new commit. Best regards, Melody