Implement logical indexing for SubArray. Fixes #4763

[magistere]

No description provided.

[ivarne]

This does only work for 1d subarrays. I don't know how to generalize it to higher dimensions (yet).

[timholy]

Generalizing to higher dimensions could be done using the same trick as in array.jl (where it looks like this came from), but it won't be efficient because linear indexing (while a lot better than it used it be) is still crazy-slow with SubArrays. A faster approach would use cartesian indexing, e.g. for 2d

    if I[i,j]
        out[c] = s[i,j]
        ...

One could (and probably should) hand-write versions for low-dimensional cases, but a general solution would involve gen_cartesian_map which has something of a learning curve.

Given the impending release I will allow others to decide whether to merge this. If it gets merged, another issue needs to be filed to handle setindex! and to generalize to higher dimensions.

[ivarne]

I think we should at least merge a NotImplementedError() kind of solution for all dimensions of logical indexing. The current behaviour is subtile a bug, that should not be released.

[magistere]

@timholy yes, I used the array.jl implementation as base for this.
Now Array and SubArray implementations are almost identical and I think it will be better to refactor all logical indexing into abstractarray.jl if it will not degrade performance. But we still need implementation of this for general case.

[JeffBezanson]

There is no need to write out cases for dimensions 1-5; a static parameter could be used:

getindex{T,N}(S::SubArray{T,N}, I::AbstractArray{Bool,N}) = getindex_bool_1d(S, I)

[magistere]

@JeffBezanson I thought about this definition at first too, but it does not work on 1D and 2D tests:

import Base.getindex
import Base.Test.@test

function getindex_bool_1d(S::SubArray, I::AbstractArray{Bool})
    n = sum(I)
    out = similar(S, n)
    c = 1
    for i = 1:length(I)
        if I[i]
            out[c] = S[i]
            c += 1
        end
    end
    out
end

#getindex{T}(S::SubArray{T,1}, I::AbstractArray{Bool,1}) = getindex_bool_1d(S, I)
#getindex{T}(S::SubArray{T,2}, I::AbstractArray{Bool,2}) = getindex_bool_1d(S, I)
getindex{T,N}(S::SubArray{T,N}, I::AbstractArray{Bool,N}) = getindex_bool_1d(S, I)

# 1D case
A = sub([1:10], 5:8)
@test A[A.<7] == [5, 6]

# 2D case
B = reshape(1:16, 4, 4)
Bs = sub(B, 2:3, 2:3)
@test Bs[Bs.>8] == [10, 11]

Output:

Warning: New definition
    getindex(SubArray{T,N,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},AbstractArray{Bool,N}) at R:\Julia\bugs\filter_subarray.jl:19
is ambiguous with:
    getindex(SubArray{T,1,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},AbstractArray{S<:Integer,1}) at subarray.jl:273.
To fix, define
    getindex(SubArray{T,1,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},AbstractArray{Bool,1})
before the new definition.
Warning: New definition
    getindex(SubArray{T,N,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},AbstractArray{Bool,N}) at R:\Julia\bugs\filter_subarray.jl:19
is ambiguous with:
    getindex(SubArray{T,N,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},Union(Real,AbstractArray{T,1})...) at subarray.jl:349.
To fix, define
    getindex(SubArray{T,N,A<:AbstractArray{T,N},I<:(Union(Int32,Range{Int32},Ran
ge1{Int32})...,)},AbstractArray{Bool,N})
before the new definition.
ERROR: test failed: :((A[(A.<7)]==[5,6]))
 in error at error.jl:21
 in default_handler at test.jl:19
 in do_test at test.jl:39
 in include at boot.jl:238
 in include_from_node1 at loading.jl:114
 in process_options at client.jl:303
 in _start at client.jl:389
at R:\Julia\bugs\filter_subarray.jl:23

[timholy]

@JeffBezanson, if I understand your suggestion correctly (and I may not be), the main problem is that it will stink from a performance perspective. Currently it's pretty important to use cartesian indexing rather than linear indexing when dealing with multidimensional SubArrays.