Switch to FFTA

[andreasnoack]

This drops a binary GPL dependency while potentially adds support for more element types as FFTA is a generic, but still quite fast, FFT implementation in Julia. For small problems, there is a minor slowdown, but it goes away for slightly larger problems where the processing between the ffts starts dominating.

Closes #123

Timings

# FFTA
julia> @btime kde(x) setup=(x=randn(100));
  31.458 μs (49 allocations: 86.75 KiB)

julia> @btime kde(x) setup=(x=randn(1_000_000));
  58.692 ms (51 allocations: 7.72 MiB)

julia> @btime kde(x) setup=(x=randn(100, 2));
  3.168 ms (171 allocations: 7.53 MiB)

julia> @btime kde(x) setup=(x=randn(1_000_000, 2));
  134.598 ms (176 allocations: 38.08 MiB)


# FFTW
julia> @btime kde(x) setup=(x=randn(100))
  37.375 μs (39 allocations: 54.27 KiB)

julia> @btime kde(x) setup=(x=randn(1_000_000))
  61.329 ms (41 allocations: 7.69 MiB)

julia> @btime kde(x) setup=(x=randn(100, 2))
  814.625 μs (57 allocations: 2.55 MiB)

julia> @btime kde(x) setup=(x=randn(1_000_000, 2));
  121.698 ms (62 allocations: 33.11 MiB)

[devmotion]

I'm in favor of this change. I think this is a better approach than #117 since users and downstream dependencies such as plotting packages would not have to choose an FFT backend, so this PR would not affect their workflow/implementation.

I'm using KernelDensity mostly indirectly in plotting packages, and there the performance regression for small arrays is not relevant. I wonder if there are applications/downstream packages for which it could matter?

Since FFTA is not as mature as eg. FFTW yet - can we expect any further performance improvements in the future?

[andreasnoack]

Since FFTA is not as mature as eg. FFTW yet - can we expect any further performance improvements in the future?

There are several improvements in the pipeline. The real FFT can be made faster, see JuliaMath/FFTA.jl#98, which I have now updated the PR with. See the timings for the 1D case which are now faster than FFTW. Once there is a new release of FFTA, I'll suggest that we move forward with this one. Then I'll promise to have the (less common) 2D case ready before too long.

[devmotion]

Is the conversion needed? Does it fix a type inference problem?

[andreasnoack]

See JuliaMath/FFTA.jl#78. It is caused by AbstractFFTs.

[devmotion]

I see now that k.density is hardcoded to Matrix{Float64} (and similarly to Vector{Float64} in the univariate case below). Initially, I was worried that this change could lead to different return types and additional allocations.

Could we avoid the need for the explicit Matrix conversion by specifying the region explicitly or using an in-place method?

Alternatively, I think we should change the conversion to a type assertion to make it clear that there are no additional allocations or type changes here and to avoid silently introducing those in case we generalize the type of k.density at some point.

[andreasnoack]

Could we avoid the need for the explicit Matrix conversion by specifying the region explicitly or using an in-place method?

Possibly, but with the current hard-coding of k.density, I think a type assert is the simplest solution (I've updated the PR). I hope that the inference issue will be fixed upstream, at which point the type assertion can just be removed.