Rules for FFT

[jessebett]

It appears there are adjoint rules for FFT in Zygote here: https://github.com/FluxML/Zygote.jl/pull/215

[jessebett]

Here is the relevant file in Zygote src: https://github.com/FluxML/Zygote.jl/blob/a1e3f9ffbf49fd710ab2f117e3ee4138a6677aa7/src/lib/array.jl#L539-L585

[roflmaostc]

Hey, today I was missing a rule for fft!. The easiest way is to add that missing lines directly to Zygote, however in the long run it would be better to invest some work so that it's based on ChainRules.

Should the rules sit here or in AbstractFFTs?

@oxinabox What do you think about that?

[devmotion]

Apparently @stevengj suggested they should be moved to AbstractFFTs: https://github.com/JuliaMath/AbstractFFTs.jl/issues/47#issuecomment-741914478 and https://github.com/FluxML/Zygote.jl/issues/835#issuecomment-741902634

[roflmaostc]

Thanks a lot for the useful links. I'll check it out whether I can come up with a PR fixing that issue

[mcabbott]

Note that AbstractFFTs seems to be a much smaller, and much slower-changing, package than ChainRulesCore. Especially if you include a test dep on ChainRulesTestUtilititesEtc, which is also changing. So I'd still vote that they should move here.

julia> @time using ChainRulesCore
  0.210714 seconds (621.77 k allocations: 37.128 MiB, 1.89% gc time)

julia> @time using AbstractFFTs
  0.008525 seconds (18.83 k allocations: 1.371 MiB)

julia> 0.21 / 0.0085
24.705882352941174

[oxinabox]

Note that AbstractFFTs seems to be a much smaller, and much slower-changing, package than ChainRulesCore. Especially if you include a test dep on ChainRulesTestUtilititesEtc, which is also changing. So I'd still vote that they should move here.

I think they should just wait for ChainRulesCore 1.0 to be tagged, which is not long away and then they can be in AbstractFFTs. ChainRulesCore 1.0 will be much lighter (we are doing something that julia apparently is really bad at precompiling, we will stop doing that), and also will be stable. Aim is to have ChainRulesCore 1.0 done by JuliaCon

[roflmaostc]

Well, technically I can work on that PR because if people decide to move it elsewhere we already would have the code then. (Basically it's a translation from Zygote.@adjoint to rrule)

[devmotion]

I made a PR for AbstractFFTs: https://github.com/JuliaMath/AbstractFFTs.jl/pull/58