-
Notifications
You must be signed in to change notification settings - Fork 153
fix: regression in non-fast scalar indexing support #760
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #760 +/- ##
==========================================
+ Coverage 89.58% 90.07% +0.49%
==========================================
Files 11 12 +1
Lines 1008 1038 +30
==========================================
+ Hits 903 935 +32
+ Misses 105 103 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ext/ForwardDiffGPUArraysCoreExt.jl
Outdated
idxs = collect( | ||
Iterators.drop(ForwardDiff.structural_eachindex(result), offset) | ||
)[1:chunksize] | ||
result[idxs] .= partial_fn.(Ref(dual), 1:chunksize) |
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.
Does this not have an inference issue due to losing static information about size? I would think this needs to be ntuple
unless it can prove things about size.
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.
It would still be type-stable, it would just have dynamism in the function that would slow it down a bit during the broadcast.
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.
Here the chunksize is already an Int, so I don't think we will have any benefit of using an ntuple
Noted in #759 (comment), GPU is completely untested in ForwardDiff.jl, so this sets up the buildkite pipeline. I setup the backend and all, and just took a few tests from #760 to seed it. The point of this isn't really to be a comprehensive set of GPU tests but rather to update this repo to have the standard tools the other repos have so GPU doesn't regress again/more.
19e8423
to
da2efb7
Compare
26c1ec9
to
c4c62a4
Compare
Co-authored-by: David Widmann <[email protected]>
Co-authored-by: David Widmann <[email protected]>
2536221
to
11540a0
Compare
fixes #759
ForwardDiff.gradient
now supports GPU Arrayscc @ChrisRackauckas @devmotion