From 0a0394224f494b72cb738369bcbda3ba60cbfc6d Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sun, 9 Jul 2017 08:02:21 -0500 Subject: [PATCH] Store indices in CartesianRange The main advantage of this change is that you no longer lose type information from `CartesianRange(indices(A))`. The types of the specific AbstractUnitRanges are important for `similar`, etc. --- NEWS.md | 6 ++ base/deprecated.jl | 7 +++ base/multidimensional.jl | 117 ++++++++++++++++++++++---------------- base/permuteddimsarray.jl | 2 +- base/range.jl | 4 ++ test/arrayops.jl | 7 ++- test/simdloop.jl | 15 ++--- 7 files changed, 95 insertions(+), 63 deletions(-) diff --git a/NEWS.md b/NEWS.md index 11ed91cdeae6b..5892b0d882f8f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -49,6 +49,11 @@ This section lists changes that do not have deprecation warnings. * The format for a `ClusterManager` specifying the cookie on the command line is now `--worker=`. `--worker ` will not work as it is now an optional argument. + * The representation of `CartesianRange` has changed to a + tuple-of-AbstractUnitRanges; the `start` and `stop` fields are no + longer present. Use `first(R)` and `last(R)` to obtain + start/stop. ([#20974]) + Library improvements -------------------- @@ -916,6 +921,7 @@ Command-line option changes [#20609]: https://github.com/JuliaLang/julia/issues/20609 [#20889]: https://github.com/JuliaLang/julia/issues/20889 [#20952]: https://github.com/JuliaLang/julia/issues/20952 +[#20974]: https://github.com/JuliaLang/julia/issues/20974 [#21183]: https://github.com/JuliaLang/julia/issues/21183 [#21359]: https://github.com/JuliaLang/julia/issues/21359 [#21692]: https://github.com/JuliaLang/julia/issues/21692 diff --git a/base/deprecated.jl b/base/deprecated.jl index 978892ad82732..67eb714916627 100644 --- a/base/deprecated.jl +++ b/base/deprecated.jl @@ -1543,7 +1543,14 @@ end @deprecate cat_t{N,T}(::Type{Val{N}}, ::Type{T}, A, B) cat_t(Val(N), T, A, B) false @deprecate reshape{N}(A::AbstractArray, ::Type{Val{N}}) reshape(A, Val(N)) +function CartesianRange{N}(start::CartesianIndex{N}, stop::CartesianIndex{N}) + inds = map((f,l)->f:l, start.I, stop.I) + depwarn("the internal representation of CartesianRange has changed, use CartesianRange($inds) (or other more approriate AbstractUnitRange type) instead.", :CartesianRange) + CartesianRange(inds) +end + # END 0.7 deprecations # BEGIN 1.0 deprecations + # END 1.0 deprecations diff --git a/base/multidimensional.jl b/base/multidimensional.jl index 2936bd1794e82..40d7cc225cf28 100644 --- a/base/multidimensional.jl +++ b/base/multidimensional.jl @@ -2,7 +2,7 @@ ### Multidimensional iterators module IteratorsMD - import Base: eltype, length, size, start, done, next, last, in, getindex, + import Base: eltype, length, size, start, done, next, first, last, in, getindex, setindex!, IndexStyle, min, max, zero, one, isless, eachindex, ndims, iteratorsize, convert @@ -135,7 +135,6 @@ module IteratorsMD # Iteration """ - CartesianRange(Istart::CartesianIndex, Istop::CartesianIndex) -> R CartesianRange(sz::Dims) -> R CartesianRange(istart:istop, jstart:jstop, ...) -> R @@ -166,28 +165,46 @@ module IteratorsMD CartesianIndex{3}((2, 2, 2)) ``` """ - struct CartesianRange{I<:CartesianIndex} - start::I - stop::I + struct CartesianRange{N,R<:NTuple{N,AbstractUnitRange{Int}}} + indices::R end - CartesianRange(index::CartesianIndex) = CartesianRange(one(index), index) - CartesianRange(::Tuple{}) = CartesianRange{CartesianIndex{0}}(CartesianIndex{0}(()),CartesianIndex{0}(())) - CartesianRange(sz::NTuple{N,Int}) where {N} = CartesianRange(CartesianIndex(sz)) - CartesianRange(rngs::NTuple{N,Union{Integer,AbstractUnitRange}}) where {N} = - CartesianRange(CartesianIndex(map(first, rngs)), CartesianIndex(map(last, rngs))) - - convert(::Type{NTuple{N,UnitRange{Int}}}, R::CartesianRange{CartesianIndex{N}}) where {N} = - map((f,l)->f:l, first(R).I, last(R).I) - convert(::Type{NTuple{N,UnitRange}}, R::CartesianRange) where {N} = - convert(NTuple{N,UnitRange{Int}}, R) - convert(::Type{Tuple{Vararg{UnitRange{Int}}}}, R::CartesianRange{CartesianIndex{N}}) where {N} = + CartesianRange(::Tuple{}) = CartesianRange{0,typeof(())}(()) + CartesianRange(inds::NTuple{N,AbstractUnitRange{Int}}) where {N} = + CartesianRange{N,typeof(inds)}(inds) + CartesianRange(inds::Vararg{AbstractUnitRange{Int},N}) where {N} = + CartesianRange(inds) + CartesianRange(inds::NTuple{N,AbstractUnitRange{<:Integer}}) where {N} = + CartesianRange(map(r->convert(AbstractUnitRange{Int}, r), inds)) + CartesianRange(inds::Vararg{AbstractUnitRange{<:Integer},N}) where {N} = + CartesianRange(inds) + + CartesianRange(index::CartesianIndex) = CartesianRange(index.I) + CartesianRange(sz::NTuple{N,<:Integer}) where {N} = CartesianRange(map(Base.OneTo, sz)) + CartesianRange(inds::NTuple{N,Union{<:Integer,AbstractUnitRange{<:Integer}}}) where {N} = + CartesianRange(map(i->first(i):last(i), inds)) + + convert(::Type{Tuple{}}, R::CartesianRange{0}) = () + convert(::Type{NTuple{N,AbstractUnitRange{Int}}}, R::CartesianRange{N}) where {N} = + R.indices + convert(::Type{NTuple{N,AbstractUnitRange}}, R::CartesianRange{N}) where {N} = + convert(NTuple{N,AbstractUnitRange{Int}}, R) + convert(::Type{NTuple{N,UnitRange{Int}}}, R::CartesianRange{N}) where {N} = + UnitRange{Int}.(convert(NTuple{N,AbstractUnitRange}, R)) + convert(::Type{NTuple{N,UnitRange}}, R::CartesianRange{N}) where {N} = + UnitRange.(convert(NTuple{N,AbstractUnitRange}, R)) + convert(::Type{Tuple{Vararg{AbstractUnitRange{Int}}}}, R::CartesianRange{N}) where {N} = + convert(NTuple{N,AbstractUnitRange{Int}}, R) + convert(::Type{Tuple{Vararg{AbstractUnitRange}}}, R::CartesianRange) = + convert(Tuple{Vararg{AbstractUnitRange{Int}}}, R) + convert(::Type{Tuple{Vararg{UnitRange{Int}}}}, R::CartesianRange{N}) where {N} = convert(NTuple{N,UnitRange{Int}}, R) convert(::Type{Tuple{Vararg{UnitRange}}}, R::CartesianRange) = convert(Tuple{Vararg{UnitRange{Int}}}, R) - ndims(R::CartesianRange) = length(R.start) - ndims(::Type{CartesianRange{I}}) where {I<:CartesianIndex} = length(I) + ndims(R::CartesianRange) = ndims(typeof(R)) + ndims(::Type{CartesianRange{N}}) where {N} = N + ndims(::Type{CartesianRange{N,TT}}) where {N,TT} = N eachindex(::IndexCartesian, A::AbstractArray) = CartesianRange(indices(A)) @@ -201,17 +218,20 @@ module IteratorsMD @inline maxt(a::Tuple, b::Tuple{}) = a @inline maxt(a::Tuple, b::Tuple) = (max(a[1], b[1]), maxt(tail(a), tail(b))...) - eltype(::Type{CartesianRange{I}}) where {I} = I + eltype(R::CartesianRange) = eltype(typeof(R)) + eltype(::Type{CartesianRange{N}}) where {N} = CartesianIndex{N} + eltype(::Type{CartesianRange{N,TT}}) where {N,TT} = CartesianIndex{N} iteratorsize(::Type{<:CartesianRange}) = Base.HasShape() - @inline function start(iter::CartesianRange{<:CartesianIndex}) - if any(map(>, iter.start.I, iter.stop.I)) - return iter.stop+1 + @inline function start(iter::CartesianRange) + iterfirst, iterlast = first(iter), last(iter) + if any(map(>, iterfirst.I, iterlast.I)) + return iterlast+1 end - iter.start + iterfirst end - @inline function next(iter::CartesianRange{I}, state) where I<:CartesianIndex - state, I(inc(state.I, iter.start.I, iter.stop.I)) + @inline function next(iter::CartesianRange, state) + state, CartesianIndex(inc(state.I, first(iter).I, last(iter).I)) end # increment & carry @inline inc(::Tuple{}, ::Tuple{}, ::Tuple{}) = () @@ -223,39 +243,38 @@ module IteratorsMD newtail = inc(tail(state), tail(start), tail(stop)) (start[1], newtail...) end - @inline done(iter::CartesianRange{<:CartesianIndex}, state) = state.I[end] > iter.stop.I[end] + @inline done(iter::CartesianRange, state) = state.I[end] > last(iter.indices[end]) # 0-d cartesian ranges are special-cased to iterate once and only once - start(iter::CartesianRange{<:CartesianIndex{0}}) = false - next(iter::CartesianRange{<:CartesianIndex{0}}, state) = iter.start, true - done(iter::CartesianRange{<:CartesianIndex{0}}, state) = state + start(iter::CartesianRange{0}) = false + next(iter::CartesianRange{0}, state) = CartesianIndex(), true + done(iter::CartesianRange{0}, state) = state - size(iter::CartesianRange{<:CartesianIndex}) = map(dimlength, iter.start.I, iter.stop.I) + size(iter::CartesianRange) = map(dimlength, first(iter).I, last(iter).I) dimlength(start, stop) = stop-start+1 length(iter::CartesianRange) = prod(size(iter)) - last(iter::CartesianRange) = iter.stop + first(iter::CartesianRange) = CartesianIndex(map(first, iter.indices)) + last(iter::CartesianRange) = CartesianIndex(map(last, iter.indices)) - @inline function in(i::I, r::CartesianRange{I}) where I<:CartesianIndex - _in(true, i.I, r.start.I, r.stop.I) + @inline function in(i::CartesianIndex{N}, r::CartesianRange{N}) where {N} + _in(true, i.I, first(r).I, last(r).I) end _in(b, ::Tuple{}, ::Tuple{}, ::Tuple{}) = b @inline _in(b, i, start, stop) = _in(b & (start[1] <= i[1] <= stop[1]), tail(i), tail(start), tail(stop)) - simd_outer_range(iter::CartesianRange{CartesianIndex{0}}) = iter + simd_outer_range(iter::CartesianRange{0}) = iter function simd_outer_range(iter::CartesianRange) - start = CartesianIndex(tail(iter.start.I)) - stop = CartesianIndex(tail(iter.stop.I)) - CartesianRange(start, stop) + CartesianRange(tail(iter.indices)) end - simd_inner_length(iter::CartesianRange{<:CartesianIndex{0}}, ::CartesianIndex) = 1 - simd_inner_length(iter::CartesianRange, I::CartesianIndex) = iter.stop[1]-iter.start[1]+1 + simd_inner_length(iter::CartesianRange{0}, ::CartesianIndex) = 1 + simd_inner_length(iter::CartesianRange, I::CartesianIndex) = length(iter.indices[1]) - simd_index(iter::CartesianRange{<:CartesianIndex{0}}, ::CartesianIndex, I1::Int) = iter.start + simd_index(iter::CartesianRange{0}, ::CartesianIndex, I1::Int) = first(iter) @inline function simd_index(iter::CartesianRange, Ilast::CartesianIndex, I1::Int) - CartesianIndex((I1+iter.start[1], Ilast.I...)) + CartesianIndex((I1+first(iter.indices[1]), Ilast.I...)) end # Split out the first N elements of a tuple @@ -859,23 +878,23 @@ function copy!(dest::AbstractArray{T,N}, src::AbstractArray{T,N}) where {T,N} end @generated function copy!(dest::AbstractArray{T1,N}, - Rdest::CartesianRange{CartesianIndex{N}}, + Rdest::CartesianRange{N}, src::AbstractArray{T2,N}, - Rsrc::CartesianRange{CartesianIndex{N}}) where {T1,T2,N} + Rsrc::CartesianRange{N}) where {T1,T2,N} quote isempty(Rdest) && return dest if size(Rdest) != size(Rsrc) throw(ArgumentError("source and destination must have same size (got $(size(Rsrc)) and $(size(Rdest)))")) end - @boundscheck checkbounds(dest, Rdest.start) - @boundscheck checkbounds(dest, Rdest.stop) - @boundscheck checkbounds(src, Rsrc.start) - @boundscheck checkbounds(src, Rsrc.stop) - ΔI = Rdest.start - Rsrc.start + @boundscheck checkbounds(dest, first(Rdest)) + @boundscheck checkbounds(dest, last(Rdest)) + @boundscheck checkbounds(src, first(Rsrc)) + @boundscheck checkbounds(src, last(Rsrc)) + ΔI = first(Rdest) - first(Rsrc) # TODO: restore when #9080 is fixed # for I in Rsrc # @inbounds dest[I+ΔI] = src[I] - @nloops $N i (n->Rsrc.start[n]:Rsrc.stop[n]) begin + @nloops $N i (n->Rsrc.indices[n]) begin @inbounds @nref($N,dest,n->i_n+ΔI[n]) = @nref($N,src,i) end dest diff --git a/base/permuteddimsarray.jl b/base/permuteddimsarray.jl index 84f345d8a3417..f8058ac65e96a 100644 --- a/base/permuteddimsarray.jl +++ b/base/permuteddimsarray.jl @@ -157,7 +157,7 @@ function _copy!(P::PermutedDimsArray{T,N,perm}, src) where {T,N,perm} return P end -@noinline function _permutedims!(P::PermutedDimsArray, src, R1::CartesianRange{CartesianIndex{0}}, R2, R3, ds, dp) +@noinline function _permutedims!(P::PermutedDimsArray, src, R1::CartesianRange{0}, R2, R3, ds, dp) ip, is = indices(src, dp), indices(src, ds) for jo in first(ip):8:last(ip), io in first(is):8:last(is) for I3 in R3, I2 in R2 diff --git a/base/range.jl b/base/range.jl index 4d5bb7f342ae8..b333cf2492a5a 100644 --- a/base/range.jl +++ b/base/range.jl @@ -778,6 +778,10 @@ promote_rule(::Type{UnitRange{T1}}, ::Type{UR}) where {T1,UR<:AbstractUnitRange} convert(::Type{UnitRange{T}}, r::AbstractUnitRange) where {T<:Real} = UnitRange{T}(first(r), last(r)) convert(::Type{UnitRange}, r::AbstractUnitRange) = UnitRange(first(r), last(r)) +convert(::Type{AbstractUnitRange{T}}, r::AbstractUnitRange{T}) where {T} = r +convert(::Type{AbstractUnitRange{T}}, r::UnitRange) where {T} = convert(UnitRange{T}, r) +convert(::Type{AbstractUnitRange{T}}, r::OneTo) where {T} = convert(OneTo{T}, r) + promote_rule(::Type{StepRange{T1a,T1b}},::Type{StepRange{T2a,T2b}}) where {T1a,T1b,T2a,T2b} = StepRange{promote_type(T1a,T2a),promote_type(T1b,T2b)} convert(::Type{StepRange{T1,T2}}, r::StepRange{T1,T2}) where {T1,T2} = r diff --git a/test/arrayops.jl b/test/arrayops.jl index d8328f02b6d7c..7a189e0e90388 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -1544,9 +1544,10 @@ end @test a[1,2] == 7 @test 2*CartesianIndex{3}(1,2,3) == CartesianIndex{3}(2,4,6) - R = CartesianRange(CartesianIndex{2}(2,3),CartesianIndex{2}(5,5)) + R = CartesianRange(2:5, 3:5) @test eltype(R) <: CartesianIndex{2} @test eltype(typeof(R)) <: CartesianIndex{2} + @test eltype(CartesianRange{2}) <: CartesianIndex{2} indexes = collect(R) @test indexes[1] == CartesianIndex{2}(2,3) @test indexes[2] == CartesianIndex{2}(3,3) @@ -1568,8 +1569,8 @@ end @test @inferred(convert(NTuple{2,UnitRange}, R)) === (2:5, 3:5) @test @inferred(convert(Tuple{Vararg{UnitRange}}, R)) === (2:5, 3:5) - @test CartesianRange((3:5,-7:7)) == CartesianRange(CartesianIndex{2}(3,-7),CartesianIndex{2}(5,7)) - @test CartesianRange((3,-7:7)) == CartesianRange(CartesianIndex{2}(3,-7),CartesianIndex{2}(3,7)) + @test CartesianRange((3:5,-7:7)) == CartesianRange(3:5,-7:7) + @test CartesianRange((3,-7:7)) == CartesianRange(3:3,-7:7) end # All we really care about is that we have an optimized diff --git a/test/simdloop.jl b/test/simdloop.jl index 57be1d59bd79a..bbac5f1dfa82a 100644 --- a/test/simdloop.jl +++ b/test/simdloop.jl @@ -101,28 +101,23 @@ function simd_cartesian_range!(indexes, crng) indexes end -crng = CartesianRange(CartesianIndex{4}(2,0,1,3), - CartesianIndex{4}(4,1,1,5)) +crng = CartesianRange(2:4, 0:1, 1:1, 3:5) indexes = simd_cartesian_range!(Array{eltype(crng)}(0), crng) @test indexes == vec(collect(crng)) -crng = CartesianRange(CartesianIndex{2}(-1,1), - CartesianIndex{2}(1,3)) +crng = CartesianRange(-1:1, 1:3) indexes = simd_cartesian_range!(Array{eltype(crng)}(0), crng) @test indexes == vec(collect(crng)) -crng = CartesianRange(CartesianIndex{2}(-1,1), - CartesianIndex{2}(-1,3)) +crng = CartesianRange(-1:-1, 1:3) indexes = simd_cartesian_range!(Array{eltype(crng)}(0), crng) @test indexes == vec(collect(crng)) -crng = CartesianRange(CartesianIndex{1}(2), - CartesianIndex{1}(4)) +crng = CartesianRange(2:4) indexes = simd_cartesian_range!(Array{eltype(crng)}(0), crng) @test indexes == collect(crng) -crng = CartesianRange(CartesianIndex{0}(), - CartesianIndex{0}()) +crng = CartesianRange() indexes = simd_cartesian_range!(Array{eltype(crng)}(0), crng) @test indexes == vec(collect(crng))