Fix setindex! with SubDArray source (+ remove Julia 0.4 support) #76
Conversation
This file contains 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
This method is an optimization wherein we try to chunk accesses based upon the parent DArray's parts. The hard thing is then going backwards and trying to figure out which parts of the assignment indices need to be used in order to access those chunks. This is a four stage process that uses five different types of indices: 1. Find the indices of each portion of the DArray 2. Find the valid subset of indices of the SubArray that index into that portion 3. Find the portion of the indices for the assignment that need to be used for that subset of indices in step 2. This is the hard part. It requires creating another set of indices that represents the mask of valid indices from step 2. With those masks in hand, it's possible to reindex `I` to the indices we need. The trouble is that `setindex!` supports singleton dimensions in the source array in ways that `getindex` does not, so we need to selectively drop singleton dimensions as we restrict the indices. A final complication is that the last index can be a linear index over many indices in either the source or destination. 4. Finally, if the entire DArray chunk isn't getting used, we need to shift the indices from step 2 to refer to the local part of the DArray.
This is no longer needed -- the comment is from when I only had restrict_indices partially implemented
Also clarify the comment since I was confused upon coming back to this method a few weeks later
Both these lazy arrays are effectively generalizations of Tim's MappedArrays.jl package. Doing this generally adds a bit more difficulty in terms of element types, but that is true of the MappedArray type, too. It might be worth breaking this out into a package at some point.
As a further optimization, (at)inbounds could be added throughout the algorithm once it has received more widespread testing.
andreasnoack
force-pushed
the
mb
branch
2 times, most recently
from
f49141b to
bc1e1f5
Compare
Current coverage is 67.93% (diff: 78.00%)@@ master #76 diff @@
==========================================
Files 1 1
Lines 702 711 +9
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 480 483 +3
- Misses 222 228 +6
Partials 0 0
|
Remove 0.4 support
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.
Supersedes #74