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

Pass to plan_fft all optional keywords of NFFTPlan #25

Merged
merged 1 commit into from
Apr 20, 2017
Merged

Pass to plan_fft all optional keywords of NFFTPlan #25

merged 1 commit into from
Apr 20, 2017

Conversation

giordano
Copy link
Member

As suggested in #19, this patch allows setting in NFFTPlan optional keywords to be passed to plan_fft.

Actually, there isn't a large overall speed-up in nfft and nfft_adjoint passing from flags = FFTW.ESTIMATE (the default) to flags =FFTW.MEASURE, but according to profile the computation of the Fourier transforms isn't the most time-consuming task (which instead is convolution).

@tknopp
Copy link
Member

tknopp commented Apr 20, 2017

nice, looks good to me. Will merge once Travis is green.

@tknopp
Copy link
Member

tknopp commented Apr 20, 2017

The convolution is indeed the most computational part if the kernel is to large. Might be interesting if there are any optimizations left in the convolution part.
Is there a concrete application for which you use the NFFT?

@giordano
Copy link
Member Author

The convolution is indeed the most computational part if the kernel is to large. Might be interesting if there are any optimizations left in the convolution part.

This recent Python package (MIT-licensed) has a light-weight implementation of NFFT which is actually faster than NFFT.jl (if you sum planning + actual nfft, the nfft alone of NFFT.jl is faster, but the Python package doesn't have a planning stage): https://github.com/jakevdp/nfft There is also a notebook about the implementation, may be worth looking into it to see how this package can be improved.

Is there a concrete application for which you use the NFFT?

I don't use it, yet, but since a few months ago I'd like to see whether using NFFT in LombScargle.jl can improve performance with respect to the current implementation. This paper suggested that NFFT can be useful, but I still have to find some time to implement it (now it's thesis time...).

@robertdj
Copy link
Collaborator

I have looked into optimizing both the apodization and convolution. The main bottlenecks are the rem command used to compute indices and the type conversion from float to int.
If the index commands could be moved into the loop/array indexing, it would give boost.

@tknopp
Copy link
Member

tknopp commented Apr 20, 2017

hm, its not directly comparable what we do here and what the python package does since we have a lookup table for the window function.

Our planning function is certainly not optimized. For instance it might be possible to use a much smaller LUT.

@tknopp tknopp merged commit 0392ebb into JuliaMath:master Apr 20, 2017
@giordano giordano deleted the fftw-flags branch April 20, 2017 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants