Move type-pirating methods to FillArrays?

[jishnub]

E.g.

Zygote.jl/src/lib/array.jl

Line 783 in 17ca911

FFTW.fft(x::Fill, dims...) = FFTW.fft(collect(x), dims...)

This looks like it should belong to an extension in FillArrays.jl

[devmotion]

Seems one could exploit though that

$$ \begin{split} DFT(A)[k] &= \sum^{length(A)}_{n=1} \exp{\bigg(\frac{- 2\pi i (k - 1) (n - 1)}{length(A)}\bigg)} A[n] \\ &= \begin{dcases} N A[1] & \text{if } k = 1,\\ \frac{1 - \exp{(-2 \pi i (k- 1))}}{1 - \exp{\Big(\frac{-2 \pi i (k - 1)}{length(A)}\Big)}} A[1] & \text{if } k > 1, \end{dcases} \\ &= \begin{dcases} N A[1] & \text{if } k = 1,\\ 0 & \text{if } k > 1, \end{dcases} \end{split} $$

if all entries of $A$ are equal.

Edit: Simplified even more (see below).

[jishnub]

Yes, indeed, but won't the second case be zero for integer 1 < k <= length(A)? A constant array should only have the 0-frequency component populated.

[devmotion]

Yep, I just noticed that I could have simplified it even more 😅

[ToucheSir]

You somehow discovered a very old version of that file. The current Zygote codebase doesn't use FFTW at all: https://github.com/FluxML/Zygote.jl/blob/master/src/lib/array.jl

[devmotion]

It's still there, just using AbstractFFT instead of FFTW (which I assume one would also want to use in an extension rather than FFTW):

Zygote.jl/src/lib/array.jl

Lines 672 to 677 in e2d7839

	AbstractFFTs.fft(x::Fill, dims...) = AbstractFFTs.fft(collect(x), dims...)
	AbstractFFTs.bfft(x::Fill, dims...) = AbstractFFTs.bfft(collect(x), dims...)
	AbstractFFTs.ifft(x::Fill, dims...) = AbstractFFTs.ifft(collect(x), dims...)
	AbstractFFTs.rfft(x::Fill, dims...) = AbstractFFTs.rfft(collect(x), dims...)
	AbstractFFTs.irfft(x::Fill, d, dims...) = AbstractFFTs.irfft(collect(x), d, dims...)
	AbstractFFTs.brfft(x::Fill, d, dims...) = AbstractFFTs.brfft(collect(x), d, dims...)

[ToucheSir]

Right, but for that we already have #835 no? AbstractFFTs shouldn't really be a dep of Zygote in the first place.

[devmotion]

IMO it's related to #835 but not a duplicate (I guess one could view it differently though): #835 is about reverse mode rules whereas this issue is just about blatant type piracy without involving any rules. It just happens that the same package (AbstractFFTs) is involved in both.

[ToucheSir]

Yes, my point is that removing the rules for AbstractFFTs from Zygote should also involve removing any piracy (because there should be no code referring to AbstractFFTs left in Zygote afterwards).

[dlfivefifty]

Yes make it an extension . But shouldn’t it return a OneHotVector ?

[devmotion]

That was our point above 🙂