Skip to content
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

slice leads to broken indexing #2644

Closed
timholy opened this issue Mar 22, 2013 · 2 comments
Closed

slice leads to broken indexing #2644

timholy opened this issue Mar 22, 2013 · 2 comments
Labels
needs decision A decision on this change is needed

Comments

@timholy
Copy link
Member

timholy commented Mar 22, 2013

I haven't played much with subarrays and slice until very recently. Unfortunately, I've discovered some problems (present in both 0.1 and 0.2):

julia> A = rand(3, 5, 8);

julia> sA = slice(A, 2, 1:5, 1:8);

julia> size(sA)
(5,8)

julia> sA[:,3]
ERROR: reshape: invalid dimensions
 in reshape at array.jl:128
 in ref at subarray.jl:224

julia> ssA = sub(sA, 1:5, 3)
1x5 SubArray of 3x5x8 Float64 Array:
 0.21088  0.894002  0.0296009  0.58458  0.63469

julia> A[2, 1:5, 3]
1x5 Float64 Array:
 0.21088  0.894002  0.0296009  0.58458  0.63469

Obviously the getindex/ref behavior is a problem. With sub, the values are correct, but really it should be a 5 SubArray rather than a 1x5 SubArray, because the parent is two dimensional and we're snipping out a column. Also note a curiosity:

julia> sA.indexes
(2,1:5,1:8)

julia> ssA.indexes
(2:2,1:5,3)

and of course

julia> sA.strides
2-element Int64 Array:
  3
 15

In thinking through how to fix this, I came to the conclusion that there's another problem: currently there no "immediate" way to link the dimensions in indexes to the dimensions in strides. You can figure it out, but it takes some computation.

I'm inclined to suggest that we do away with the indexes field altogether and compute everything from strides and first_index, basically treating the parent as if it's just a linear buffer. If the user really needs to know how a subarray would be created from the parent, we could provide a function that computes those indexes (plus a BitArray containing information about whether singleton dimensions would have to be dropped). But for general array indexing I suspect the indexes field is really just a distraction tempting one into delivering the wrong answer.

If we do that, how much would it break? I don't see references to the indexes field outside of subarray, but of course there's a lot of code out there that I don't know about.

@timholy
Copy link
Member Author

timholy commented Mar 22, 2013

It's also worth throwing in that thinking about the possibility of a "zero-copy reshape()" on subarrays leads one to toy with some funky new range objects (e.g., that map a 2d set of strided locations to 1d). I'm not saying I want to tackle that anytime soon, but I bet it will come up eventually.

timholy added a commit that referenced this issue Mar 22, 2013
@timholy
Copy link
Member Author

timholy commented Mar 22, 2013

I was able to find a conservative fix, so I just went with that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

1 participant