Fix stride1 computation for sub. Fixes #14509

[timholy]

I think this fixes #14509 in general, although the logic is complex, and the fact that we test only up to 3 dimensions could bite us. There's an alternative way of fixing this, described below, that would be more robust, but it risks breaking any package that dives into the internal representation of SubArrays.

Here's the logic of the problem and this fix:

• We use Int indexes to encode "dropped" dimensions, and UnitRange{Int} indexes to encode "retained" dimensions. sub, which does not drop anything other than trailing dimensions, therefore converts non-trailing Int indexes into UnitRange{Int}.
• Int indexes don't break uniformity of the stride, whereas UnitRange indexes might. Hence there is some conflict between marking dimensions as dropped/retained and having really good LinearFast "inference"---sub throws away useful information for the latter problem.
• LD of the parent SubArray will mark the last dimension at which your slicing did not break the uniformity of the stride. For example, for an Array A, sub(A, 2:5, 3, 3, 1:4) will have LD = 3, not LD = 1, because those intermediate Ints don't cause trouble on their own. However, the stride is 1, not 1 * size(A,2) * size(A,3) or size(A,1) * size(A,2) * size(A,3).
• When creating a view-of-a-view, if you encounter a UnitRange index, and there is another one by the time you reach the LDth index, you can infer that this is not the one that "broke" uniformity of the stride. In such cases, the stride should be something like size(A,1) * size(A,2) * size(A,3).

The more robust way to fix this would be to end the conflict between encoding dropped/retained dimensions and linear-indexing-inference: internally define

immutable IntWrapper
    i::Int
end

and have sub(A, 2, 1:3) return a SubArray{T, 2, typeof(A), Tuple{IntWrapper, UnitRange{Int}}, LD}.

I wonder whether this PR is the way to fix 0.4, but to use IntWrapper for 0.5? Comments would be appreciated.

[timholy]

Since this fixes a bug, I'll merge in a day or so unless I hear otherwise.

[tkelman]

This started causing freezes in the test suite on the centos 7 buildbots, so I won't be backporting this until that gets resolved: http://buildbot.e.ip.saba.us:8010/builders/build_centos7.1-x64/builds/75

[timholy]

I haven't seen such logs before, so I don't have a baseline. Is this likely a timeout? (Can these tests time out?) Has it been close to the limit in the past? I could have the subarray tests make some noise, if that's all it would take.

Note that the code change here is tiny; it's most likely that doubling the number of tests is what's causing the problem.

[tkelman]

I take that back, it looks like this failure mode started a little earlier: http://buildbot.e.ip.saba.us:8010/builders/build_centos7.1-x64?numbuilds=250 - subarray tests look like they finish in most of those logs, but the tests don't finish doing the last few: parallel, compile.

[timholy]

OK, definitely keep me posted if you go back to suspecting this to be a problem.

[mbauman]

This seems to have introduced a failure into the full test suite:

$ JULIA_TESTFULL=1 make -C test subarray
    JULIA test/subarray
     * subarray             I = ([1],1,1)
(ld,ldc) = (3,0)
ERROR: LoadError: LoadError: Linear indexing inference mismatch
 in err_li at /Users/mbauman/Code/julia-0.4/test/subarray.jl:199
 in runtests at /Users/mbauman/Code/julia-0.4/test/subarray.jl:215
 in runviews at /Users/mbauman/Code/julia-0.4/test/subarray.jl:292

[timholy]

Argh. That's a case where linear indexing happens to work out for the particular values supplied, but doesn't work for arbitrary choices with the same types. So in this case it's the test that's in error.

I'm running TESTFULL with

ld = single_stride_dim(C)

replaced with

ld = min(single_stride_dim(C), dim_break_linindex(I))

where

function dim_break_linindex(I)
    i = 1
    while i <= length(I) && !isa(I[i], Vector{Int})
        i += 1
    end
    i - 1
end

I'll likely head to bed before it finishes, so I wanted you to know what I'm trying in case you'd prefer to just launch your own run.

[mbauman]

Thanks for digging into this! I had a hunch it was a test issue, but I didn't look too closely.

[timholy]

Yep, that fixes it for me. Explicitly, here's the patch:

$ git diff
diff --git a/test/subarray.jl b/test/subarray.jl
index 5e342e9..8bd62b0 100644
--- a/test/subarray.jl
+++ b/test/subarray.jl
@@ -207,10 +207,18 @@ function err_li(S::SubArray, ld::Int, szC)
     error("Linear indexing inference mismatch")
 end

+function dim_break_linindex(I)
+    i = 1
+    while i <= length(I) && !isa(I[i], Vector{Int})
+        i += 1
+    end
+    i - 1
+end
+
 function runtests(A::Array, I...)
     # Direct test of linear indexing inference
     C = Agen_nodrop(A, I...)
-    ld = single_stride_dim(C)
+    ld = min(single_stride_dim(C), dim_break_linindex(I))
     ldc = Base.subarray_linearindexing_dim(typeof(A), typeof(I))
     ld == ldc || err_li(I, ld, ldc)
     # sub
@@ -245,7 +253,7 @@ function runtests(A::SubArray, I...)
     AA = copy_to_array(A)
     # Direct test of linear indexing inference
     C = Agen_nodrop(AA, I...)
-    Cld = ld = single_stride_dim(C)
+    Cld = ld = min(single_stride_dim(C), dim_break_linindex(I))
     Cdim = AIindex = 0
     while Cdim <= Cld && AIindex < length(A.indexes)
         AIindex += 1

Feel free to include in the PR you're working on, or let me know and I'll push it.