Skip to content

Commit

Permalink
Simpler and more systematic index conversions
Browse files Browse the repository at this point in the history
This patch dramatically simplifies the fallback indexing algorithms for arrays
by standardizing the way in which the passed indices are converted `to_index`
and by rigorously defining what a supported index type is.

The very first thing that occurs within the fallback indexing methods is that
the passed indices are converted to a supported indexing type by
`to_indices(...)`. A supported index type is either `Int` or an `AbstractArray`
of indices. This means that it is at this point that both `Colon` and arrays of
booleans get converted to collections of indices (as specialized array types:
`Slice` and `LogicalIndex`, respectively). This *drastically* simplifies much
of the internal logic. Whereas we had previously encoded the special behaviors
of `:` indexing in about four places, this patch does it once and then stores
the result in a `Slice` AbstractVector. Similarly, `CartesianIndex` is now
converted within `to_indices`, as well.

In addition to simplifying the internal definitions, this patch also makes it
easier for both external packages and Base to add new behaviors in a systematic
fashion. Simple extensions would include things like `Not` indices and
[EndpointRanges.jl](https://github.com/JuliaArrays/EndpointRanges.jl).

Additionally, logical indexing is now represented by a specialized
`LogicalIndex` type that enables fast iteration over the boolean mask without
allocations. This allows index conversions to occur before checking bounds,
and should enable further optimizations for LinearSlow arrays.

 ### Breaking Behaviors:

* The stored type of indices in SubArrays change from `Colon` to `Slice`.
  • Loading branch information
mbauman committed Dec 28, 2016
1 parent e81d03b commit 2cc158b
Show file tree
Hide file tree
Showing 12 changed files with 362 additions and 373 deletions.
97 changes: 47 additions & 50 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ false
checkindex(::Type{Bool}, inds::AbstractUnitRange, i) = throw(ArgumentError("unable to check bounds for indices of type $(typeof(i))"))
checkindex(::Type{Bool}, inds::AbstractUnitRange, i::Real) = (first(inds) <= i) & (i <= last(inds))
checkindex(::Type{Bool}, inds::AbstractUnitRange, ::Colon) = true
checkindex(::Type{Bool}, inds::AbstractUnitRange, ::Slice) = true
function checkindex(::Type{Bool}, inds::AbstractUnitRange, r::Range)
@_propagate_inbounds_meta
isempty(r) | (checkindex(Bool, inds, first(r)) & checkindex(Bool, inds, last(r)))
Expand Down Expand Up @@ -769,7 +770,6 @@ end
pointer{T}(x::AbstractArray{T}) = unsafe_convert(Ptr{T}, x)
pointer{T}(x::AbstractArray{T}, i::Integer) = (@_inline_meta; unsafe_convert(Ptr{T},x) + (i-first(linearindices(x)))*elsize(x))


## Approach:
# We only define one fallback method on getindex for all argument types.
# That dispatches to an (inlined) internal _getindex function, where the goal is
Expand All @@ -782,61 +782,62 @@ pointer{T}(x::AbstractArray{T}, i::Integer) = (@_inline_meta; unsafe_convert(Ptr

function getindex(A::AbstractArray, I...)
@_propagate_inbounds_meta
_getindex(linearindexing(A), A, I...)
error_if_canonical_indexing(linearindexing(A), A, I...)
_getindex(linearindexing(A), A, to_indices(A, I)...)
end
function unsafe_getindex(A::AbstractArray, I...)
@_inline_meta
@inbounds r = getindex(A, I...)
r
end

error_if_canonical_indexing(::LinearFast, A::AbstractArray, ::Int) = error("indexing not defined for ", typeof(A))
error_if_canonical_indexing{T,N}(::LinearSlow, A::AbstractArray{T,N}, ::Vararg{Int, N}) = error("indexing not defined for ", typeof(A))
error_if_canonical_indexing(::LinearIndexing, ::AbstractArray, ::Any...) = nothing

## Internal definitions
_getindex(::LinearIndexing, A::AbstractArray, I...) = error("indexing $(typeof(A)) with types $(typeof(I)) is not supported")

## LinearFast Scalar indexing: canonical method is one Int
_getindex(::LinearFast, A::AbstractVector, ::Int) = error("indexing not defined for ", typeof(A))
_getindex(::LinearFast, A::AbstractArray, ::Int) = error("indexing not defined for ", typeof(A))
_getindex(::LinearFast, A::AbstractVector, i::Int) = (@_propagate_inbounds_meta; getindex(A, i))
_getindex(::LinearFast, A::AbstractArray, i::Int) = (@_propagate_inbounds_meta; getindex(A, i))
_getindex{T}(::LinearFast, A::AbstractArray{T,0}) = A[1]
_getindex(::LinearFast, A::AbstractArray, i::Real) = (@_propagate_inbounds_meta; getindex(A, to_index(i)))
function _getindex{T,N}(::LinearFast, A::AbstractArray{T,N}, I::Vararg{Real,N})
function _getindex{T,N}(::LinearFast, A::AbstractArray{T,N}, I::Vararg{Int,N})
# We must check bounds for sub2ind; so we can then use @inbounds
@_inline_meta
J = to_indexes(I...)
@boundscheck checkbounds(A, J...)
@inbounds r = getindex(A, sub2ind(A, J...))
@boundscheck checkbounds(A, I...)
@inbounds r = getindex(A, sub2ind(A, I...))
r
end
function _getindex(::LinearFast, A::AbstractVector, I1::Real, I::Real...)
function _getindex(::LinearFast, A::AbstractVector, I1::Int, I::Int...)
@_inline_meta
J = to_indexes(I1, I...)
@boundscheck checkbounds(A, J...)
@inbounds r = getindex(A, J[1])
@boundscheck checkbounds(A, I1, I...)
@inbounds r = getindex(A, I1)
r
end
function _getindex(::LinearFast, A::AbstractArray, I::Real...) # TODO: DEPRECATE FOR #14770
function _getindex(::LinearFast, A::AbstractArray, I::Int...) # TODO: DEPRECATE FOR #14770
@_inline_meta
J = to_indexes(I...)
@boundscheck checkbounds(A, J...)
@inbounds r = getindex(A, sub2ind(A, J...))
@boundscheck checkbounds(A, I...)
@inbounds r = getindex(A, sub2ind(A, I...))
r
end


## LinearSlow Scalar indexing: Canonical method is full dimensionality of Ints
_getindex{T,N}(::LinearSlow, A::AbstractArray{T,N}, ::Vararg{Int, N}) = error("indexing not defined for ", typeof(A))
_getindex{T,N}(::LinearSlow, A::AbstractArray{T,N}, I::Vararg{Real, N}) = (@_propagate_inbounds_meta; getindex(A, to_indexes(I...)...))
function _getindex(::LinearSlow, A::AbstractArray, i::Real)
_getindex{T,N}(::LinearSlow, A::AbstractArray{T,N}, I::Vararg{Int, N}) = (@_propagate_inbounds_meta; getindex(A, I...))
function _getindex(::LinearSlow, A::AbstractArray, i::Int)
# ind2sub requires all dimensions to be > 0; may as well just check bounds
@_inline_meta
@boundscheck checkbounds(A, i)
@inbounds r = getindex(A, ind2sub(A, to_index(i))...)
@inbounds r = getindex(A, ind2sub(A, i)...)
r
end
@generated function _getindex{T,AN}(::LinearSlow, A::AbstractArray{T,AN}, I::Real...) # TODO: DEPRECATE FOR #14770
@generated function _getindex{T,AN}(::LinearSlow, A::AbstractArray{T,AN}, I::Int...) # TODO: DEPRECATE FOR #14770
N = length(I)
if N > AN
# Drop trailing ones
Isplat = Expr[:(I[$d]) for d = 1:AN]
Osplat = Expr[:(to_index(I[$d]) == 1) for d = AN+1:N]
Osplat = Expr[:(I[$d] == 1) for d = AN+1:N]
quote
@_propagate_inbounds_meta
@boundscheck (&)($(Osplat...)) || throw_boundserror(A, I)
Expand All @@ -848,7 +849,7 @@ end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=max(N,1):AN]
szcheck = Expr[:(size(A, $d) > 0) for d=max(N,1):AN]
last_idx = N > 0 ? :(to_index(I[$N])) : 1
last_idx = N > 0 ? :(I[$N]) : 1
quote
# ind2sub requires all dimensions to be > 0:
@_propagate_inbounds_meta
Expand All @@ -862,7 +863,8 @@ end
# function that allows dispatch on array storage
function setindex!(A::AbstractArray, v, I...)
@_propagate_inbounds_meta
_setindex!(linearindexing(A), A, v, I...)
error_if_canonical_indexing(linearindexing(A), A, I...)
_setindex!(linearindexing(A), A, v, to_indices(A, I)...)
end
function unsafe_setindex!(A::AbstractArray, v, I...)
@_inline_meta
Expand All @@ -873,49 +875,44 @@ end
_setindex!(::LinearIndexing, A::AbstractArray, v, I...) = error("indexing $(typeof(A)) with types $(typeof(I)) is not supported")

## LinearFast Scalar indexing
_setindex!(::LinearFast, A::AbstractVector, v, ::Int) = error("indexed assignment not defined for ", typeof(A))
_setindex!(::LinearFast, A::AbstractArray, v, ::Int) = error("indexed assignment not defined for ", typeof(A))
_setindex!(::LinearFast, A::AbstractVector, v, i::Int) = (@_propagate_inbounds_meta; setindex!(A, v, i))
_setindex!(::LinearFast, A::AbstractArray, v, i::Int) = (@_propagate_inbounds_meta; setindex!(A, v, i))
_setindex!{T}(::LinearFast, A::AbstractArray{T,0}, v) = (@_propagate_inbounds_meta; setindex!(A, v, 1))
_setindex!(::LinearFast, A::AbstractArray, v, i::Real) = (@_propagate_inbounds_meta; setindex!(A, v, to_index(i)))
function _setindex!{T,N}(::LinearFast, A::AbstractArray{T,N}, v, I::Vararg{Real,N})
function _setindex!{T,N}(::LinearFast, A::AbstractArray{T,N}, v, I::Vararg{Int,N})
# We must check bounds for sub2ind; so we can then use @inbounds
@_inline_meta
J = to_indexes(I...)
@boundscheck checkbounds(A, J...)
@inbounds r = setindex!(A, v, sub2ind(A, J...))
@boundscheck checkbounds(A, I...)
@inbounds r = setindex!(A, v, sub2ind(A, I...))
r
end
function _setindex!(::LinearFast, A::AbstractVector, v, I1::Real, I::Real...)
function _setindex!(::LinearFast, A::AbstractVector, v, I1::Int, I::Int...)
@_inline_meta
J = to_indexes(I1, I...)
@boundscheck checkbounds(A, J...)
@inbounds r = setindex!(A, v, J[1])
@boundscheck checkbounds(A, I1, I...)
@inbounds r = setindex!(A, v, I1)
r
end
function _setindex!(::LinearFast, A::AbstractArray, v, I::Real...) # TODO: DEPRECATE FOR #14770
function _setindex!(::LinearFast, A::AbstractArray, v, I::Int...) # TODO: DEPRECATE FOR #14770
@_inline_meta
J = to_indexes(I...)
@boundscheck checkbounds(A, J...)
@inbounds r = setindex!(A, v, sub2ind(A, J...))
@boundscheck checkbounds(A, I...)
@inbounds r = setindex!(A, v, sub2ind(A, I...))
r
end

# LinearSlow Scalar indexing
_setindex!{T,N}(::LinearSlow, A::AbstractArray{T,N}, v, ::Vararg{Int, N}) = error("indexed assignment not defined for ", typeof(A))
_setindex!{T,N}(::LinearSlow, A::AbstractArray{T,N}, v, I::Vararg{Real, N}) = (@_propagate_inbounds_meta; setindex!(A, v, to_indexes(I...)...))
function _setindex!(::LinearSlow, A::AbstractArray, v, i::Real)
_setindex!{T,N}(::LinearSlow, A::AbstractArray{T,N}, v, I::Vararg{Int, N}) = (@_propagate_inbounds_meta; setindex!(A, v, I...))
function _setindex!(::LinearSlow, A::AbstractArray, v, i::Int)
# ind2sub requires all dimensions to be > 0; may as well just check bounds
@_inline_meta
@boundscheck checkbounds(A, i)
@inbounds r = setindex!(A, v, ind2sub(A, to_index(i))...)
@inbounds r = setindex!(A, v, ind2sub(A, i)...)
r
end
@generated function _setindex!{T,AN}(::LinearSlow, A::AbstractArray{T,AN}, v, I::Real...) # TODO: DEPRECATE FOR #14770
@generated function _setindex!{T,AN}(::LinearSlow, A::AbstractArray{T,AN}, v, I::Int...) # TODO: DEPRECATE FOR #14770
N = length(I)
if N > AN
# Drop trailing ones
Isplat = Expr[:(I[$d]) for d = 1:AN]
Osplat = Expr[:(to_index(I[$d]) == 1) for d = AN+1:N]
Osplat = Expr[:(I[$d] == 1) for d = AN+1:N]
quote
# We only check the trailing ones, so just propagate @inbounds state
@_propagate_inbounds_meta
Expand All @@ -928,7 +925,7 @@ end
sz = Expr(:tuple)
sz.args = Expr[:(size(A, $d)) for d=max(N,1):AN]
szcheck = Expr[:(size(A, $d) > 0) for d=max(N,1):AN]
last_idx = N > 0 ? :(to_index(I[$N])) : 1
last_idx = N > 0 ? :(I[$N]) : 1
quote
# ind2sub requires all dimensions to be > 0:
@_propagate_inbounds_meta
Expand Down Expand Up @@ -964,7 +961,7 @@ function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::Union{Range, Abstract
X
end

get(A::AbstractArray, I::Range, default) = get!(similar(A, typeof(default), index_shape(A, I)), A, I, default)
get(A::AbstractArray, I::Range, default) = get!(similar(A, typeof(default), index_shape(I)), A, I, default)

# TODO: DEPRECATE FOR #14770 (just the partial linear indexing part)
function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::RangeVecIntList, default::T)
Expand All @@ -974,7 +971,7 @@ function get!{T}(X::AbstractArray{T}, A::AbstractArray, I::RangeVecIntList, defa
X
end

get(A::AbstractArray, I::RangeVecIntList, default) = get!(similar(A, typeof(default), index_shape(A, I...)), A, I, default)
get(A::AbstractArray, I::RangeVecIntList, default) = get!(similar(A, typeof(default), index_shape(I...)), A, I, default)

## structured matrix methods ##
replace_in_print_matrix(A::AbstractMatrix,i::Integer,j::Integer,s::AbstractString) = s
Expand Down Expand Up @@ -1729,7 +1726,7 @@ function mapslices(f, A::AbstractArray, dims::AbstractVector)
idx = Any[first(ind) for ind in indices(A)]
itershape = tuple(dimsA[otherdims]...)
for d in dims
idx[d] = Colon()
idx[d] = Slice(indices(A, d))
end

Aslice = A[idx...]
Expand Down
16 changes: 8 additions & 8 deletions base/array.jl
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
typealias AbstractVector{T} AbstractArray{T,1}
typealias AbstractMatrix{T} AbstractArray{T,2}
typealias AbstractVecOrMat{T} Union{AbstractVector{T}, AbstractMatrix{T}}
typealias RangeIndex Union{Int, Range{Int}, AbstractUnitRange{Int}, Colon}
typealias RangeIndex Union{Int, Range{Int}, AbstractUnitRange{Int}}
typealias DimOrInd Union{Integer, AbstractUnitRange}
typealias IntOrInd Union{Int, AbstractUnitRange}
typealias DimsOrInds{N} NTuple{N,DimOrInd}
Expand Down Expand Up @@ -448,8 +448,8 @@ done(a::Array,i) = (@_inline_meta; i == length(a)+1)
## Indexing: getindex ##

# This is more complicated than it needs to be in order to get Win64 through bootstrap
getindex(A::Array, i1::Real) = arrayref(A, to_index(i1))
getindex(A::Array, i1::Real, i2::Real, I::Real...) = (@_inline_meta; arrayref(A, to_index(i1), to_index(i2), to_indexes(I...)...)) # TODO: REMOVE FOR #14770
getindex(A::Array, i1::Int) = arrayref(A, i1)
getindex(A::Array, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayref(A, i1, i2, I...)) # TODO: REMOVE FOR #14770

# Faster contiguous indexing using copy! for UnitRange and Colon
function getindex(A::Array, I::UnitRange{Int})
Expand All @@ -472,13 +472,13 @@ function getindex(A::Array, c::Colon)
end

# This is redundant with the abstract fallbacks, but needed for bootstrap
function getindex{S,T<:Real}(A::Array{S}, I::Range{T})
return S[ A[to_index(i)] for i in I ]
function getindex{S}(A::Array{S}, I::Range{Int})
return S[ A[i] for i in I ]
end

## Indexing: setindex! ##
setindex!{T}(A::Array{T}, x, i1::Real) = arrayset(A, convert(T,x)::T, to_index(i1))
setindex!{T}(A::Array{T}, x, i1::Real, i2::Real, I::Real...) = (@_inline_meta; arrayset(A, convert(T,x)::T, to_index(i1), to_index(i2), to_indexes(I...)...)) # TODO: REMOVE FOR #14770
setindex!{T}(A::Array{T}, x, i1::Int) = arrayset(A, convert(T,x)::T, i1)
setindex!{T}(A::Array{T}, x, i1::Int, i2::Int, I::Int...) = (@_inline_meta; arrayset(A, convert(T,x)::T, i1, i2, I...)) # TODO: REMOVE FOR #14770

# These are redundant with the abstract fallbacks but needed for bootstrap
function setindex!(A::Array, x, I::AbstractVector{Int})
Expand Down Expand Up @@ -1312,7 +1312,7 @@ function find(testf::Function, A)
return I
end
_index_remapper(A::AbstractArray) = linearindices(A)
_index_remapper(iter) = Colon() # safe for objects that don't implement length
_index_remapper(iter) = OneTo(typemax(Int)) # safe for objects that don't implement length

"""
find(A)
Expand Down
33 changes: 33 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1144,6 +1144,39 @@ eval(Base.Dates, quote
recur{T<:TimeType}(fun::Function, start::T, stop::T; step::Period=Day(1), negate::Bool=false, limit::Int=10000) = recur(fun, start:step:stop; negate=negate)
end)

# Index conversions revamp; #19730
function getindex(A::LogicalIndex, i::Int)
depwarn("indexing into logical indices is deprecated; use iteration or collect the indices into an array instead.", :getindex)
checkbounds(A, i)
first(Iterators.drop(A, i-1))
end
function to_indexes(I...)
depwarn("to_indexes is deprecated; pass both the source array and tuple of indices to `to_indices(A, I)` instead.", :to_indexes)
map(_to_index, I)
end
_to_index(i) = to_index(I)
_to_index(c::Colon) = c
function getindex(::Colon, i)
depwarn("indexing into Colons is deprecated; convert Colons to Slices with to_indices", :getindex)
to_index(i)
end
function unsafe_getindex(::Colon, i::Integer)
depwarn("indexing into Colons is deprecated; convert Colons to Slices with to_indices", :unsafe_getindex)
to_index(i)
end
function step(::Colon)
depwarn("step(::Colon) is deprecated; convert Colons to Slices with to_indices", :step)
1
end
function isempty(::Colon)
depwarn("isempty(::Colon) is deprecated; convert Colons to Slices with to_indices", :isempty)
false
end
function in(::Integer, ::Colon)
depwarn("in(::Integer, ::Colon) is deprecated; convert Colons to Slices with to_indices", :in)
true
end

# #18931
@deprecate cummin(A, dim=1) accumulate(min, A, dim)
@deprecate cummax(A, dim=1) accumulate(max, A, dim)
Expand Down
7 changes: 7 additions & 0 deletions base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,13 @@ function isassigned(v::SimpleVector, i::Int)
end

# index colon
"""
Colon()
Colons (:) are used to signify indexing entire objects or dimensions at once.
Very few operations are defined on Colons directly; instead they are converted
to `Slice`s upon indexing (within `to_indices`).
"""
immutable Colon
end
const (:) = Colon()
Expand Down
Loading

0 comments on commit 2cc158b

Please sign in to comment.