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.

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

Add NewRecur for Blocked RNNs #2316

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mkschleg
Copy link
Contributor

@mkschleg mkschleg commented Aug 24, 2023

This adds the NewRecur functionality from Fluxperimental.jl. There are some road bumps that need to be ironed out.

  1. The rrule for scan_full needs to be made type stable. I thought this had to do w/ the hobbits being a vector of Any, but when I separated out the first element and the other elements (this is where the issue is) I still see type instability when using Cthulu. I've included the new rrule in this pull request and below it is a commented out version of the Fluxperimental.jl one. I'm not sure where else to look for type instability as the stack to call this is pretty deep. I think at some point the type instability starts coming from Zygote.ZBacks in the stack. (I need help knowing how to iterate on this more).
  2. Naming scheme. Does it make sense to name this NewRecur, or should we try and make this the standard for v0.15 while deprecating the old recur in v0.14 and changing the name? Another idea is to follow Lux.jl and call this Recurrent.
  3. Tests. I have the tests I made in Fluxperimental for this, but maybe there are more we should do?

PR Checklist

  • Tests are added
  • Entry in NEWS.md
  • Documentation, if applicable

@ToucheSir
Copy link
Member

@darsnack and I had a long discussion about this on last week's ML call, and came to the conclusion that it would really help to co-develop this with the apply functionality in FluxML/Fluxperimental.jl#7. Since the latter isn't fully implemented yet and the interplay between the two isn't fully established, it does make sense to keep prototyping and testing both in Fluxperimental. That would also let us kick tasks such as figuring out type stability further down the road.

Now, we also discussed a lot of ideas on what the eventual APIs should look like as a coherent whole. At some point this should be turned into a writeup, but since it's mostly nascent ideas we thought it might help to have a call to flesh out some of the details. Let us know on Zulip/Slack if you're interested and we can work out the logistics :)

@ToucheSir ToucheSir mentioned this pull request Feb 12, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants