-
Notifications
You must be signed in to change notification settings - Fork 42
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
Upgrade ADTypes to v1 #295
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #295 +/- ##
=========================================
+ Coverage 0.00% 0.06% +0.06%
=========================================
Files 24 25 +1
Lines 1366 1463 +97
=========================================
+ Hits 0 1 +1
- Misses 1366 1462 +96 ☔ View full report in Codecov by Sentry. |
@ChrisRackauckas this is ready for a first pass. Not sure if the missed lines in Codecov are my fault or not. |
Looks good! |
"Pankaj Mishra <[email protected]>", | ||
"Chris Rackauckas <[email protected]>", | ||
] | ||
version = "2.19.0" |
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.
Can we do a major version bump here for safety since we are dropping ADTypes pre-1.0?
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.
I know ColPrac doesn't require it, but NonlinearSolve is very tightly coupled rn with sparsedifftools..
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.
I was wondering whether to include more changes to SparseDiffTools first. Basically I only did the backend part, but the new ADTypes also has infrastructure (supertypes and functions) for sparsity detection + coloring, which SparseDiffTools could extend. On the other hand that would be super super breaking, which is why I didn't do it in the current PR (my priority was getting things to work)
Upgrade ADTypes to v1.0 for the sparse backend types only (nothing yet about the coloring and sparsity detection infrastructure).
Supersedes #294
Main changes
AutoSparseForwardDiff(...)
(construction) withAutoSparse(AutoForwardDiff(...))
AutoSparseForwardDiff
(dispatch) withAutoSparse{<:AutoForwardDiff}
(be careful with type parameters like{C}
)AutoSparseEnzyme
anymoresparse_jacobian_cache(ad, args...; kwargs...) = sparse_jacobian_cache_aux(mode(ad), ad, args...; kwargs...)
. Add implem forForwardOrReverseMode
which checks if we're dealing withDiffractor
orEnzyme
(not hit by tests for some reason 🤔)_get_colorvec(alg, ad)
with_get_colorvec(alg, mode(ad))
Questions