-
Notifications
You must be signed in to change notification settings - Fork 13
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
First test scenarios for Flux gradients #352
Conversation
Thanks so much for your contribution! It may take a few days but I'll get back to you, give you some feedback on the code and take it from here if that's okay. I'm sorry that the task was so daunting, but very grateful that you agreed to give it a try :) And if you need any more resources on AD, let me know |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #352 +/- ##
==========================================
+ Coverage 96.57% 96.58% +0.01%
==========================================
Files 98 99 +1
Lines 4813 4888 +75
==========================================
+ Hits 4648 4721 +73
- Misses 165 167 +2 ☔ View full report in Codecov by Sentry. |
CI
DIT source
DIT extensions
DIT tests
DI extensions
DI tests
|
@CarloLucibello I got the first batch of tests working with Zygote. The syntax is a bit counter-intuitive because DI doesn't yet support multiple arguments, so I have to put the data |
Actually this is probably a case of exactly the opposite in which doing so would be harmful. Marking the closure as differentiable would mean that Enzyme would also end up computing the derivative of the input [and then throwing it away]. |
Per the error message, it looks like the closure is causing issues where it can't be compiled due to mixed activity [e.g. one element of the closure being differentiable the other not]. As the error message states, you can enable runtime activity to resolve at a slight [but not asymptotic performance loss]. But also using multiple arguments here in Enzyme directly would also presumably resolve this |
I tried enabling runtime activity but the stack traces just got angrier 😱
Agreed, but setting this up in the single-argument case also helped me resolve some other issues to pave the way for non-array testing, so it's a great first step. For the multi-argument case, what I need most of all is inspiration and ideas to improve my initial API proposal at #311 (comment). The idea is to be slightly different than Enzyme, and closer to the mathematical concepts that users may be familiar with. |
Merging this without the Enzyme tests because there's already some useful stuff in there. Billy if you have a clue what the problem was in https://github.com/gdalle/DifferentiationInterface.jl/actions/runs/9963932557/job/27531092106 with runtime activity switched on, I'd love to hear your thoughts |
Work done during JuliaCon Hackathon; preparing test for neural networks