-
Notifications
You must be signed in to change notification settings - Fork 35
Add Enzyme tests #860
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
Comments
Duplicate of #739 We already have a PR opened against Enzyme: EnzymeAD/Enzyme.jl#1813. We can simply mirror the test set. It’s important to run Enzyme test as an isolated integration test. |
As I explained elsewhere, I don't think Enzyme is mature enough:
Some of these issues can be fixed by adding integration tests on both sides (Enzyme + DynamicPPL), but lots of other issues can't and require significant improvements to Enzyme itself. |
The error was a type instability in this repo........... for which adding some tests would help give some notion of assurances, right? In any case, if you have any issues open an issue. I'm unaware of any segfaults from turing, or issues with numerics. Adding integration tests (as proposed here), would help find/fix these, yes? |
Enzyme tests against Bijectors and AdvancedVI are constantly broken. Some issues have been reported on Enzyme.jl. Due to the frequency of Enzyme failures, it has now become a maintenance burden. Enzyme's fragility, coupled with the difficulty of debugging Enzyme failures, means that CI failures about Enzyme are often ignored.
My point is that libraries like Turing.jl should not be responsible for discovering and reporting Enzyme bugs. Asking for more integration tests on Turing.jl will help Enzyme, but it would also add more maintenance burden for us. It is more of the autodiff dev's responsibility to catch bugs via better unit and integration tests. A more systematic approach to bug fixing will help the Enzyme dev team. |
now that the type instability is fixed, it would probably be useful to add Enzyme tests to make sure there isn't another regression in the future (and also give some assurances of support to users)
The text was updated successfully, but these errors were encountered: