WIP: convolve_fft memory & execution time profiling - #4374

...this clearly needs a rebase.

Here are some plots from an example run. They're a bit confusing; the tables are a little easier to parse. The timing is not monotonic with size because FFTs are faster for 2^n sizes.

These figures are a little more readable and show the interesting spikes you can get without fft_pad. I used a finer grid of image sizes for this.

This one should be very useful (BUT NOTE THE AXIS LABELS ARE SWAPPED!):

I'll try to populate it with a bigger parameter space, but I'm a little worried about crashing my computer.

Well, my tests worked, but my ipython session did eventually crash. Still, recovered the results:

So the rule of thumb is: always fft_pad if you can, but sometimes you can't! In the True,True case, the green line cuts off early because I did not run the tests when the 1GB array size warning was raised.

@embray would you prefer these plots in the docs, on the wiki, or somewhere else? I think they're useful. They've been run on a late 2013 macbook pro with 16 GB RAM; it would be interesting to run them elsewhere. Also, it would be really interesting to see if there's any difference in performance with the type of fft used (np.fft, fftw3, etc.).

@keflavich - This will need a changelog entry, too.

This won't make the 2.0 release; I've removed the milestone. This PR is really just a tool to assess convolution speeds. It is useful, but it needs to be incorporated into the docs.

OK, sorry. I saw that you've just rebased it, thus the assumption.

The rebase was to see if it could be included. Upon further review, nope!

This would be a cool student project if anyone were ever inclined... it's not of the scale of a GSoC, but maybe a good 1-week "learn about convolution" project.

I've cancelled the osx build.

There seems to be genuine issues with the optional dependencies that needs to be addressed, so I've cancelled some travis jobs.

The built docs will appear here eventually.

Hi humans 👋 - this pull request hasn't had any new commits for approximately 5 months. I plan to close this in a month if the pull request doesn't have any new commits by then.

In lieu of a stalled pull request, please consider closing this and open an issue instead if a reminder is needed to revisit in the future. Maintainers may also choose to add keep-open label to keep this PR open but it is discouraged unless absolutely necessary.

If this PR still needs to be reviewed, as an author, you can rebase it to reset the clock. You may also consider sending a reminder e-mail about it to the astropy-dev mailing list.

If you believe I commented on this pull request incorrectly, please report this here.

This is one of those perpetually-too-low-priority projects. It would have been appropriate to finish it for the astropy paper, but if this gets autoclosed, it's OK.

Btw, I was recently comparing performance for cubes convolution with scipy and pyfftw (which is multi-threaded but not so easy to use), and found that numpy caches the computed ffts (less than 100Mb and 32 items). Did you take this into account for your time measurements ? (Looking quickly at the code, it seems not).

Arguably v3.1 will be about performance, so ultimately it would be an ideal release for this.

⏰ Time's up! ⏰

I'm going to close this pull request as per my previous message. If you think what is being added/fixed here is still important, please remember to open an issue to keep track of it. Thanks!

If this is the first time I am commenting on this issue, or if you believe I closed this issue incorrectly, please report this here.

Apparently I built the docs associated with this a few years ago:

https://keflavich-astropy.readthedocs.io/en/convolve_fft_profiling/convolution/performance.html

@keflavich - if you have time to resurrect this for 3.2, I suppose it would be more than welcome improvement.