Don't persist Zygote backend cache#76
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #76 +/- ##
==========================================
+ Coverage 84.36% 84.68% +0.31%
==========================================
Files 8 8
Lines 467 470 +3
==========================================
+ Hits 394 398 +4
+ Misses 73 72 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
ToucheSir
force-pushed
the
zygote-nocache
branch
from
5fc01e5 to
ced0448
Compare
There was a problem hiding this comment.
Could we overload AD.pullback_function(::ReverseRuleConfig{<:Zygote.ZygoteRuleConfig}, f, xs...) instead of making ZygoteRuleConfig a separate backend? I think it is very unfortunatem and unintuitive if ReverseRuleBackend(ZygoteRuleConfig()) is broken and behaves differently from ZygoteBackend.
Edit: I opened #77 to show what I had in mind.
|
|
||
| AD.ZygoteBackend() = AD.ReverseRuleConfigBackend(Zygote.ZygoteRuleConfig()) | ||
| AD.@primitive function pullback_function(ba::AD.ZygoteBackend, f, xs...) | ||
| @inline # @primitive doesn't support this before function keyword |
There was a problem hiding this comment.
This syntax is not supported on Julia 1.6.
There was a problem hiding this comment.
| @inline # @primitive doesn't support this before function keyword |
There was a problem hiding this comment.
That's odd, I thought I force-pushed a new commit which removed it. Either way, #77 is the way to go.
| To be able to use this backend, you have to load Zygote. | ||
| """ | ||
| function ZygoteBackend end | ||
| struct ZygoteBackend <: AbstractReverseMode end |
There was a problem hiding this comment.
This also requires changing the docstring and the README which both state that it is just a special ReverseRuleConfigBackend.
|
I thought about that, but somehow got it in my mind that it would require an ugly |
2 participants
Fixes #69 and closes #70.