Repository navigation
DArray: Add accumulate, cumsum and cumprod - #775
Merged
Merged
Conversation
There was no `accumulate` method for a `DArray`, so `Base`'s generic one
scanned it element by element. With `dims` that throws a
`ConcurrencyViolationError`, and on a vector it returns a `DVector{Any}`.
Scan each partition on its own, then combine the running last slice of
the partitions before it along `dims` into it. Only the carries run in
sequence, and each one is a single slice. Like `reduce`, this assumes
`op` is associative. Only the first partition starts from `init`, so when
`init` widens the eltype the partitions are widened before the scan.
OpenCL.jl has no `accumulate` for `CLArray`, so `Base` falls back to scalar indexing. Metal has no Float64, so the `init=0.0` test can't run there.
Contributor
Author
|
Pushed a fix for the OpenCL and Metal jobs. OpenCL.jl has no The other red jobs look unrelated, most of them fail on master too. |
Member
|
Thanks so much! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #770.
accumulatehad noDArraymethod, soBasewalked it element by element. This adds one, andcumsum/cumproduse it.Each partition is scanned on its own, then the last slice of the partitions before it is carried in along
dims. The carries are one slice each, so only they run in order and the scans stay parallel. Likereducethis needs an associativeop, soaccumulate(-, DA)won't matchBase.One catch: only the first partition gets
init, so withInt8data andinit=0the later partitions overflow if you do nothing about it:So when
initwidens the eltype, the partitions get widened before the scan. There's a test for that now.array/mapreducepasses on 1.10, 1.11 and 1.12 (484 tests), and the gpu suite on a Quadro RTX 8000 (248). With the change reverted the new tests fail on both.Rough timing on 8 threads (the machine was busy):
cumsum(DA; dims=1)on 8192x8192 takes about 61 ms, vs 305 ms forBaseon the sameArray.cumsum!/accumulate!into an existingDArraystill fail, I left those for later. Also added the three to the supported list in the docs.