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

Replace np.fft routines with pyFFTW #20

Closed
mrava87 opened this issue Jan 8, 2019 · 3 comments

Comments

@mrava87
Copy link
Collaborator

commented Jan 8, 2019

For speeding up routines involving fft pyFFTW (https://github.com/pyFFTW/pyFFTW) may be a good choice.

It comes with an extra dependency and will leave the user with the manual installation of FFTW as described in pyFFTW manual, but it has wrappers to numpy/scipy fits, so we could fallback to bumpy if FFTW is not available.

@mrava87 mrava87 added the enhancement label Jan 8, 2019

@prisae

This comment has been minimized.

Copy link
Contributor

commented Jan 10, 2019

If I am not mistaken FFTW is not in scipy because of the license. So you will have to make the user install it on its own and provide a fallback-option, I guess. Not sure though.

@mrava87

This comment has been minimized.

Copy link
Collaborator Author

commented Jan 11, 2019

Correct user will have to install FFTW. My initial thoughts are that more HPC oriented libraries (pyfftw, numba, pycuda, Dask) will be eventually used but we should always have fallbacks to basic libraries (numpy-scipy).

For example here we could have a private class _FFT_numpy and one _FFT_fftw and then a function FFT creating the correct operator based on user flag + check that pyfftw can be imported. Just an idea, maybe there is even a better way?

All said, the condition for pyfftw to be included is that we can have a travis build able to install FFTW and do tests on the FFT operators (that should be doable as pyfftw does it itself), if so I would say pyfftw should be used as fft is at the core of many operators (and run 100-200 times during an inversion with iterative solvers)

@mrava87

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 15, 2019

Close this as a double backend was added in this PR 91ac047

@mrava87 mrava87 closed this Feb 15, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.