Move over adjoints from Flux

FluxML / Zygote.jl

21st century AD

https://fluxml.ai/Zygote.jl/

Other

1.48k stars 213 forks source link

Move over adjoints from Flux #81

Closed MikeInnes closed 3 years ago

MikeInnes commented 5 years ago

Flux has a lot of gradient definitions that we need to port over to Zygote. Copying them over would be an easy patch for anyone interested in contributing.

DoktorMike commented 5 years ago

Good idea. Could you showcase just one such definition port? Might make it easier for someone to jump in. :)

MikeInnes commented 5 years ago

For example, matmul in Tracker and matmul in Zygote.

In general the process is: copy it over to roughly the same place in array.jl, change @grad -> @adjoint, and remove any calls to data. Let me know if there's any other questions!

c0nn3r commented 5 years ago

I'm going to give this a try, thanks for the example!

c0nn3r commented 5 years ago

Sorry, just getting to this now. I've ported over the following functions (the copypaste job is done ;) )

view
cat (might need to be changed to match how vcat and hcat are now implemented)
mean
diagm
dot
det
logdet
logabsdet
inv

Should I also port over the ones from NNLib.jl as well?

MikeInnes commented 5 years ago

There aren't any custom adjoints in NNlib, but there are some in Flux for NNlib – maybe that's what you mean. But yeah, it'd be good to have them.

staticfloat commented 5 years ago

Just a note that my branch sf/nnlib_overhaul addresses most of the NNlib adjoints, I think.

ToucheSir commented 3 years ago

Is this issue still relevant? The only remaining @adjoints I see would be xlogy (which should probably belong in NNlib if it can be rewritten as an rrule), and normalization on GPU (covered by https://github.com/FluxML/NNlib.jl/issues/19).

DhairyaLGandhi commented 3 years ago

Yeah we have basically done this one.