-
Notifications
You must be signed in to change notification settings - Fork 19
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
bump Enzyme version in v0.2 #132
Conversation
@Red-Portal Enzyme is still fragile, as observed in other parts of the TuringLang ecosystem. I don't think it is worth supporting it officially from AdvancedVI. Maybe move all Enzyme tests into a separate CI task and allow them to fail during PR checks. |
Thanks @Red-Portal, this is very helpful! The tests on Turing's master are indeed broken because of 1.11. @penelopeysm, do you think either TuringLang/Turing.jl#2341 or TuringLang/Turing.jl#2368 is close enough to fixing the test breakage that it makes sense to hold off with this one for a moment? |
@mhauru I suspect #2341 might be merged by today or tomorrow! |
@yebai Do you have a reference to anything which is fragile in other parts of turing? From my understanding TuringLang/Turing.jl#1887 has been passing all tests for the past four months without issue (though certainly there's more to do like the additional ones distributions tests @mhauru opened) |
My impression is that |
So @mhauru has been working on this recently here: EnzymeAD/Enzyme.jl#1819 as far as I can tell, its got a few more failures than the other tools, but they're all determinstic in throwing julia errors for currently unsupported distributions (and tend to be moreso forward mode, with reverse mode generally being more fine) |
Oh I wasn't aware of that. I'll keep an eye! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good to me, and the Turing integration tests now pass (thanks @penelopeysm!). The buildkite thing is failing (is this GPU tests?) but it seems to have been failing for the previous v2 release too, so maybe that's fine? Happy to merge @Red-Portal?
Merged! |
I see I arrived late by seconds :) |
This PR bumps Enzyme's compat bound to v0.13 so that AdvancedVI v0.2 doesn't hold the Turing ecosystem behind.