search

JuliaStats / KernelDensity.jl

Kernel density estimators for Julia
Other
172 stars 40 forks source link

Drop FFTW dependency #123

Open andreasnoack opened 1 month ago

andreasnoack commented 1 month ago

Since it effectively makes this package GPL licensed. Some possibilities might be

cc @devmotion

simonbyrne commented 1 month ago

AbstractFFTs.jl might be useful?

andreasnoack commented 1 month ago

Possibly but the README says that

This package is mainly not intended to be used directly.

A while ago I discussed this option a bit with @devmotion, though, and while it might provide the freedom to choose your favorite FFT package then my feeling is that most users of this package would prefer not to know about FFT packages.

devmotion commented 1 month ago

I opened a PR a while ago but the reactions where a bit mixed: https://github.com/JuliaStats/KernelDensity.jl/pull/117 The main issue with the AbstractFFTs approach taken in that PR is that as @andreasnoack said it requires users to load an FFT package (and due to the oddities of the backend selection in AbstractFFTs any (in)direct dependency loading an FFT package collides with this approach - so mixing e.g. FFTW and RustFFTs in the ecosystem would create additional problems).

Ultimately, having faced this problem multiple times my impression is that AbstractFFTs has to be redesigned such that one can select and combine different backends more easily. Moreover, for user convenience a fallback on a (non-GPL?) backend would be ideal in case packages do not define their own default backend. Maybe one could use the preference system to set global, package-specific, and user-specific default backends (in case it's desirable to not hardcode it). Unfortunately, FFTW alternatives such as RustFFTs or FFTA are not complete enough yet and therefore not a drop-in replacement for FFTW.