-
-
Notifications
You must be signed in to change notification settings - Fork 617
Enable other reactant tests #2600
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
Conversation
|
@mcabbott @ToucheSir a lot of these remaining errors seem to be bugs in the testing infra? Do you have any idea what's happening (or could help make an isolated example of what's going awry) |
|
From reading the log... error is from this utility function: Lines 38 to 44 in 0e36af9
Seems to be called with 1st argument Zygote gradient, and 2nd argument Enzyme gradient: Line 109 in 0e36af9
And Zygote will often truncate branches which are like Signature in another error is other order... but it looks like Enzyme has left Tuple{Int64, Int64} in the gradient, where Zygote has nothing. Then in the next recursion inwards, you could get |
|
yeah Enzyme keeps the primal and shadow structures the same, so it sounds like the test works -- but the test function itself is busted here? |
|
Yes, that's right. Sorry I stopped before the conclusion! Somehow the function for doing approximate equality on nested structs needs to be made smart enough to handle the difference between Zygote & Enzyme conventions. |
|
@mcabbott do you know what's needed to move the needle here? |
|
bumping again here @mcabbott @CarloLucibello |
|
Sorry I don't have bandwidth to think about this. If anyone else does, trying to simplify gradient tests would be very welcome. Zygote may be on the way out, so far there appears to be nobody trying to make it run on 1.12. Possibly working on tests and on removing this would best be done together. |
|
@mcabbott @CarloLucibello this should now pass, if it can get a review/merge? |
|
CUDA and Metal buildkite tests are failing |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2600 +/- ##
===========================================
+ Coverage 32.09% 68.47% +36.38%
===========================================
Files 32 33 +1
Lines 2038 2087 +49
===========================================
+ Hits 654 1429 +775
+ Misses 1384 658 -726 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Looks like all passes, including all Julia versions and devices |
No description provided.