Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move over adjoints from Flux #81

Closed
MikeInnes opened this issue Feb 28, 2019 · 8 comments
Closed

Move over adjoints from Flux #81

MikeInnes opened this issue Feb 28, 2019 · 8 comments
Labels
help wanted Extra attention is needed

Comments

@MikeInnes
Copy link
Member

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.
@MikeInnes MikeInnes added the help wanted Extra attention is needed label Feb 28, 2019
@DoktorMike
Copy link

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

@MikeInnes MikeInnes mentioned this issue Mar 8, 2019
Merged
9 tasks
@MikeInnes
Copy link
Member Author

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
Copy link

c0nn3r commented Mar 8, 2019

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

@MikeInnes MikeInnes mentioned this issue Mar 9, 2019
Closed
@c0nn3r
Copy link

c0nn3r commented Mar 9, 2019 •
edited
Loading

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
Copy link
Member Author

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
Copy link
Contributor

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

@mcabbott mcabbott mentioned this issue Jun 6, 2019
Merged
bors bot added a commit to FluxML/Flux.jl that referenced this issue Sep 11, 2019
@bors @MikeInnes
@staticfloat @thebhatman
Merge #669
20c0c4f 
669: using Zygote r=MikeInnes a=MikeInnes

Otherwise known as "break all the things". This will be a huge change so I'm beginning to prepare now, even though Zygote is still a couple of months off from being really ready. **Do not try this at home** (yet) – this branch is eventually aimed at beta testers, but isn't even ready for that yet.

The idea is to break as little code as possible, which means supporting the current `Params` API; but I also want to start prototyping the nicer things discussed in #628 and other issues.

Blocking issues:

* [x] Get the tests passing.
* [x] Check tests on GPU.
* [x] Rewrite all the docs.
* [x] Cache invalidation (JuliaLabs/Cassette.jl#6).
* [x] Moving over adjoints (FluxML/Zygote.jl#81).
* [x] General Zygote robustness.

Nice to have:

* [ ] Robust nested AD (may not be a blocker if one can still use Tracker with Flux).
* [x] Zygote support for modules / globals as discussed in #628, along with #637.
* [x] Better train/test mode as in #643.

If you're the kind of person who ignores triangular road signs, you can try this with

```julia
]add Flux#zygote Zygote#master
```

Co-authored-by: Mike J Innes <mike.j.innes@gmail.com>
Co-authored-by: Elliot Saba <staticfloat@gmail.com>
Co-authored-by: thebhatman <manjunathbhat9920@gmail.com>
@ToucheSir
Copy link
Member

ToucheSir commented Jun 14, 2021 •
edited
Loading

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 FluxML/NNlib.jl#19).

@DhairyaLGandhi
Copy link
Member

Yeah we have basically done this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@staticfloat @MikeInnes @ToucheSir @DoktorMike @c0nn3r @DhairyaLGandhi