Skip to content

Commit

Permalink
Merge pull request #19730 from JuliaLang/mb/too_many_indices
Browse files Browse the repository at this point in the history
RFC: Speedier, simpler and more systematic index conversions
  • Loading branch information
vchuravy authored Jan 13, 2017
2 parents 77ca345 + 1b2b738 commit 54ffc51
Show file tree
Hide file tree
Showing 14 changed files with 442 additions and 393 deletions.
103 changes: 50 additions & 53 deletions base/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,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 @@ -799,7 +800,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 @@ -812,61 +812,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 @@ -878,7 +879,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 @@ -892,7 +893,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 @@ -903,49 +905,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 @@ -958,7 +955,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 @@ -994,7 +991,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 @@ -1004,7 +1001,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 @@ -1173,8 +1170,7 @@ function cat_t(dims, T::Type, X...)
return _cat(A, shape, catdims, X...)
end

function _cat(A, shape, catdims, X...)
N = length(shape)
function _cat{N}(A, shape::NTuple{N}, catdims, X...)
offsets = zeros(Int, N)
inds = Vector{UnitRange{Int}}(N)
concat = copy!(zeros(Bool, N), catdims)
Expand All @@ -1187,7 +1183,8 @@ function _cat(A, shape, catdims, X...)
inds[i] = 1:shape[i]
end
end
A[inds...] = x
I::NTuple{N, UnitRange{Int}} = (inds...,)
A[I...] = x
end
return A
end
Expand Down Expand Up @@ -1759,7 +1756,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 @@ -1358,7 +1358,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
34 changes: 34 additions & 0 deletions base/deprecated.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1125,6 +1125,40 @@ 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("getindex(A::LogicalIndex, i) is deprecated; use iteration or index into the result of `collect(A)` 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 `A` and indices as `to_indices(A, $(I...))` instead.", :to_indexes)
map(_to_index, I)
end
_to_index(i) = to_index(I)
_to_index(c::Colon) = c
const _colon_usage_msg = "convert Colons to a set of indices for indexing into array `A` by passing them in a complete tuple of indices `I` to `to_indices(A, I)`"
function getindex(::Colon, i)
depwarn("getindex(::Colon, i) is deprecated; $_colon_usage_msg", :getindex)
to_index(i)
end
function unsafe_getindex(::Colon, i::Integer)
depwarn("getindex(::Colon, i) is deprecated; $_colon_usage_msg", :unsafe_getindex)
to_index(i)
end
function step(::Colon)
depwarn("step(::Colon) is deprecated; $_colon_usage_msg", :step)
1
end
function isempty(::Colon)
depwarn("isempty(::Colon) is deprecated; $_colon_usage_msg", :isempty)
false
end
function in(::Integer, ::Colon)
depwarn("in(::Integer, ::Colon) is deprecated; $_colon_usage_msg", :in)
true
end

# #18931
@deprecate cummin(A, dim=1) accumulate(min, A, dim)
@deprecate cummax(A, dim=1) accumulate(max, A, dim)
Expand Down
10 changes: 9 additions & 1 deletion base/essentials.jl
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,15 @@ function isassigned(v::SimpleVector, i::Int)
return x != C_NULL
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
by `to_indices` to an internal vector type (`Base.Slice`) to represent the
collection of indices they span before being used.
"""
immutable Colon
end
const (:) = Colon()
Expand Down
1 change: 1 addition & 0 deletions base/exports.jl
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ export
sum!,
sum,
sum_kbn,
to_indices,
vcat,
vec,
view,
Expand Down
Loading

0 comments on commit 54ffc51

Please sign in to comment.