-
-
Notifications
You must be signed in to change notification settings - Fork 612
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
Conversation
Minor edits, adding some notes.
@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 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 :) |
What is the status of this? |
I lost steam on working on Julia due to issues I was having w/ Autodiff/Zygote back in 2023, without any vision on when an alternative would be available. I couldn't really justify the time I was putting in anymore. From what I remember, this was in a working state and likely dealt with the gradient issues from #2185. No work was done on playing w/ apply with this interface. I expect it to be relatively straightforward though. |
I'm going to close since with the redesign in #2500 the state is handled explicitly and we don't have a Recur layer. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2316 +/- ##
==========================================
- Coverage 79.47% 7.33% -72.14%
==========================================
Files 31 31
Lines 1749 1799 +50
==========================================
- Hits 1390 132 -1258
- Misses 359 1667 +1308 ☔ View full report in Codecov by Sentry. |
I'm glad this is getting fixed. |
This adds the NewRecur functionality from Fluxperimental.jl. There are some road bumps that need to be ironed out.
scan_full
needs to be made type stable. I thought this had to do w/ the hobbits being a vector ofAny
, but when I separated out the first element and the other elements (this is where the issue is) I still see type instability when usingCthulu
. 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 fromZygote.ZBack
s in the stack. (I need help knowing how to iterate on this more).Recurrent
.PR Checklist