Zygote.jl
Zygote.jl copied to clipboard
FluxML
Remove Zygote rule for `*(::AbstractFFTs.Plan, ::AbstractArray)`
The potentially incorrect Zygote rules for FFT (https://github.com/FluxML/Zygote.jl/issues/899) can be removed now that comprehensive Chain Rules have been added in https://github.com/JuliaMath/AbstractFFTs.jl/pull/67
cc @devmotion
PR Checklist
- ~[ ] Tests are added~
- ~[ ] Documentation, if applicable~
I assume they can't be removed right now since the AbstractFFT PR is not sufficient - downstream packages have to implement the adjoint functionality first. Otherwise the rules will just error.
Indeed, tests fail currently since e.g. AbstractFFTs.ProjectionStyle is not defined for FFTW plans.
Sorry to just bump this, but I've also run into https://github.com/FluxML/Zygote.jl/issues/899 and wanted to check whether this can be solved. As https://github.com/JuliaMath/FFTW.jl/pull/249 has been merged, would it be possible to tag a patch release of FFTW and merge this?
@stevengj, @devmotion could you do this?
Sorry to just bump this, but I've also run into #899 and wanted to check whether this can be solved. As JuliaMath/FFTW.jl#249 has been merged, would it be possible to tag a patch release of FFTW and merge this?
For tests to pass with this PR, we also need an AbstractFFTs release due to https://github.com/JuliaMath/AbstractFFTs.jl/pull/116. In addition, the (half incorrect) Zygote rules that would be removed here do apply to GPU code, while support for AdjointStyle's by CUDA is bottlenecked by https://github.com/JuliaMath/AbstractFFTs.jl/pull/118.
