**BREAKING**: Move AD to package extensions and drop Zygote tests for Julia 1.12 and up#592
Conversation
6d6f4b7 to
03917d4
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #592 +/- ##
=======================================
Coverage 94.46% 94.46%
=======================================
Files 52 52
Lines 1463 1463
=======================================
Hits 1382 1382
Misses 81 81 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
theogf
left a comment
There was a problem hiding this comment.
Instead of the whole > v1.12, should we just drop Zygote support entirely? The tests are failing anyway.
|
We could do that, but my plan was to have an intermediate release where extent of breakage/support is properly documented because I feel that not having a tested successor for AD is not a very good state to be in. So 0.11 would be a transition state. I feel that removing all Zygote hacks and making sure everything works with e.g. Mooncake or Enzyme is a much larger task, perhaps for a version v1? Enzyme for example has correctness bugs related to BLAS, I don't want to say "we dropped Zygote support on all versions, use something else" when that something is likely giving wrong results. Still something I would like to address in the near term, but there could potentially be a bigger step there, rewriting tests using ADInterface and testing different backends, perhaps also GPU support with AD is now finally within reach? At least it's worth investigating I think. |
|
Yeah integrating DITests is something I want to get done at some point! Since it's just a temporary thing I guess it's fine to merge as is even if the |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
The plan would be to merge this and then release as 0.11. Would that be ok for you? |
|
KernelFunctions.jl documentation for PR #592 is available at: |
Summary
We move AD-related code to package extensions for cleaner separation of essential vs. nonessential parts of the package.
Zygote is broken on Julia 1.12, so we mark these tests as broken to have less CI noise and not miss actual breakage.
This was discussed in #585.
What alternatives have you considered?
We could have left the rules in the main package and simply marked the tests as broken. But since the AD code is not needed when one does not load an AD package that benefits from it, this is arguably cleaner.
Breaking changes
If there ever was a promise that KernelFunctions would work with Zygote, it is now broken on Julia 1.12. Zygote is still expected to work and tested on Julia LTS. The tutorial example mentions this, but right now we don't test with any other AD packages. This can be done in a future version.