splitting NuFFT into base and uu,vv cached class.

[iancze]

closes #231 when complete.

• this changes the old fourier.NuFFT -> new fourier.NuFFTCached. The __init__ call of fourier.NuFFTCached is the same as the old fourier.NuFFT. You would use it like nufft = fourier.NuFFTCached(coords=coords, nchan=1, uu=uu, vv=vv)
• creates a new fourier.NuFFT that does not require uu or vv on init. You would init it like nufft = fourier.NuFFT(coords=coords, nchan=1). The difference comes when using the forward routine, in that you now supply u and v points. E.g., nufft(image_cube(), uu, vv)
• The idea is that the new fourier.NuFFT should be the go-to routine for most use cases. The fourier.NuFFTCached should be used when one expects to make multiple calls to forward using the exact same set of uu and vv points (but different input images).

[jeffjennings]

• It looks like the pyro tutorial has an outdated called to the nufft.

• The naming for nufftcached and nufft makes sense, but I would suggest not reusing the name of an old class for a new class with a different functionality and/or call signature. Yes the code is pre-1.0 and you want the new nufft to be the default, but this breaks backwards compatability and will probably be frustrating for some users. Instead, the cached implementation could still be NuFFT and the new could be NuFFTdynamic or something. If the new names are kept as is, I'd suggest the changelog note that the NuFFT is not backwards compatible as of the next release version.

[iancze]

Thanks for the review!

• It looks like the pyro tutorial has an outdated called to the nufft.

Ah, right. This is in the "large-tutorials" whose cache is not cleared with a make clean, so I missed it.

• The naming for nufftcached and nufft makes sense, but I would suggest not reusing the name of an old class for a new class with a different functionality and/or call signature. Yes the code is pre-1.0 and you want the new nufft to be the default, but this breaks backwards compatability and will probably be frustrating for some users. Instead, the cached implementation could still be NuFFT and the new could be NuFFTdynamic or something. If the new names are kept as is, I'd suggest the changelog note that the NuFFT is not backwards compatible as of the next release version.

I agree the change of names is unfortunate, but I think the original implementation and naming was enough of a mistake that it should be corrected (especially since we are pre-1.0, the user base is small, and we should take these opportunities for re-organization/consolidation). The new NuFFT should be the preferred behavior in most cases (and more closely matches the original TorchKbNuFFT pattern), whereas the caching behavior is much more specialized. Honestly, for ALMA-sized datasets, I'm not even sure if there will be many cases where NuFFT cached will be used. It's prohibitively slow as the number of visibility points grows, so predicting full datasets will be necessary using the behaviour of the new NuFFT. I'll make an entry in the changelog about the switch.