From 315d348ee62ad7212ff3d7e34253c70723ac1a31 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 21:28:32 -0400 Subject: [PATCH 01/15] more powerful reverse(A; dims) --- base/abstractarraymath.jl | 50 ----------------- base/array.jl | 24 ++++++-- base/arraymath.jl | 114 +++++++++++++++++++------------------- base/bitarray.jl | 9 +-- base/range.jl | 6 +- 5 files changed, 82 insertions(+), 121 deletions(-) diff --git a/base/abstractarraymath.jl b/base/abstractarraymath.jl index f68ff82cd61f6..953c190ab12ef 100644 --- a/base/abstractarraymath.jl +++ b/base/abstractarraymath.jl @@ -127,56 +127,6 @@ julia> selectdim(A, 2, 3) return view(A, idxs...) end -""" - reverse(A; dims::Integer) - -Reverse `A` in dimension `dims`. - -# Examples -```jldoctest -julia> b = [1 2; 3 4] -2×2 Matrix{Int64}: - 1 2 - 3 4 - -julia> reverse(b, dims=2) -2×2 Matrix{Int64}: - 2 1 - 4 3 -``` -""" -function reverse(A::AbstractArray; dims::Integer) - nd = ndims(A); d = dims - 1 ≤ d ≤ nd || throw(ArgumentError("dimension $d is not 1 ≤ $d ≤ $nd")) - if isempty(A) - return copy(A) - elseif nd == 1 - return reverse(A) - end - inds = axes(A) - B = similar(A) - nnd = 0 - for i = 1:nd - nnd += Int(length(inds[i])==1 || i==d) - end - indsd = inds[d] - sd = first(indsd)+last(indsd) - if nnd==nd - # reverse along the only non-singleton dimension - for i in indsd - B[i] = A[sd-i] - end - return B - end - let B=B # workaround #15276 - alli = [ axes(B,n) for n in 1:nd ] - for i in indsd - B[[ n==d ? sd-i : alli[n] for n in 1:nd ]...] = selectdim(A, d, i) - end - end - return B -end - function circshift(a::AbstractArray, shiftamt::Real) circshift!(similar(a), a, (Integer(shiftamt),)) end diff --git a/base/array.jl b/base/array.jl index 0205d96b3e449..2014c784b3df6 100644 --- a/base/array.jl +++ b/base/array.jl @@ -1589,22 +1589,33 @@ julia> reverse(A, 3, 5) 3 ``` """ -function reverse(A::AbstractVector, s=first(LinearIndices(A)), n=last(LinearIndices(A))) +function reverse(A::AbstractVector, start::Integer, stop::Integer=lastindex(A)) + s, n = Int(start), Int(stop) B = similar(A) - for i = first(LinearIndices(A)):s-1 + for i = firstindex(A):s-1 B[i] = A[i] end for i = s:n B[i] = A[n+s-i] end - for i = n+1:last(LinearIndices(A)) + for i = n+1:lastindex(A) B[i] = A[i] end return B end -# to resolve ambiguity with reverse(A; dims) -reverse(A::Vector) = invoke(reverse, Tuple{AbstractVector}, A) +# 1d special cases of reverse(A; dims) and reverse!(A; dims): +for (f,_f) in ((:reverse,:_reverse), (:reverse!,:_reverse!)) + @eval begin + $f(A::AbstractVector; dims=:) = $_f(A, dims) + $_f(A::AbstractVector, ::Colon) = $f(A, firstindex(A), lastindex(A)) + $_f(A::AbstractVector, dim::Tuple{Integer}) = $_f(A, first(dim)) + function $_f(A::AbstractVector, dim::Integer) + dim == 1 || throw(ArgumentError("invalid dimension $dim ≠ 1")) + return $_f(A, :) + end + end +end function reverseind(a::AbstractVector, i::Integer) li = LinearIndices(a) @@ -1637,7 +1648,8 @@ julia> A 1 ``` """ -function reverse!(v::AbstractVector, s=first(LinearIndices(v)), n=last(LinearIndices(v))) +function reverse!(v::AbstractVector, start::Integer, stop::Integer=lastindex(v)) + s, n = Int(start), Int(stop) liv = LinearIndices(v) if n <= s # empty case; ok elseif !(first(liv) ≤ s ≤ last(liv)) diff --git a/base/arraymath.jl b/base/arraymath.jl index 12931b75eb333..968bd61b3c35c 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -58,69 +58,71 @@ end ## data movement ## -reverse(A::Array; dims::Integer) = _reverse_int(A, Int(dims)) -function _reverse_int(A::Array{T}, d::Int) where T - nd = ndims(A) - 1 ≤ d ≤ nd || throw(ArgumentError("dimension $d is not 1 ≤ $d ≤ $nd")) - sd = size(A, d) - if sd == 1 || isempty(A) - return copy(A) - end +""" + reverse(A; dims=:) - B = similar(A) +Reverse `A` along dimension `dims`, which can be an integer (a +single dimension), a tuple of integers (a tuple of dimensions) +or `:` (reverse along all the dimensions, the default). See +also [`reverse!`](@ref) for in-place reversal. - nnd = 0 - for i = 1:nd - nnd += size(A,i)==1 || i==d - end - if nnd==nd - # reverse along the only non-singleton dimension - for i = 1:sd - B[i] = A[sd+1-i] - end - return B - end +# Examples +```jldoctest +julia> b = Int64[1 2; 3 4] +2×2 Matrix{Int64}: + 1 2 + 3 4 + +julia> reverse(b, dims=2) +2×2 Matrix{Int64}: + 2 1 + 4 3 - d_in = size(A) - M = 1 - for i = 1:d-1 - M *= d_in[i] + julia> reverse(b) +2×2 Array{Int64,2}: + 4 3 + 2 1 +``` +""" +reverse(A::AbstractArray; dims=:) = _reverse(A, dims) +_reverse(A, dims) = reverse!(copymutable(A); dims) + +""" + reverse!(A; dims=:) + +Like [`reverse`](@ref), but operates in-place in `A`. +""" +reverse!(A::AbstractArray; dims=:) = _reverse!(A, dims) +_reverse!(A::AbstractArray{<:Any,N}, ::Colon) where {N} = _reverse!(A, ntuple(identity, Val{N}())) +_reverse!(A, dim::Integer) = _reverse!(A, (Int(dim),)) +_reverse!(A, dims::NTuple{M,Integer}) where {M} = _reverse!(A, Int.(dims)) +function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} + if N < M || !allunique(dims) || !all(d -> 1 ≤ d ≤ N, dims) + throw(ArgumentError("invalid dimensions $dims in reverse!")) end - N = length(A) - stride = M * sd - - if M==1 - for j = 0:stride:(N-stride) - for i = 1:sd - ri = sd+1-i - B[j + ri] = A[j + i] - end - end - else - if allocatedinline(T) && M>200 - for i = 1:sd - ri = sd+1-i - for j=0:stride:(N-stride) - offs = j + 1 + (i-1)*M - boffs = j + 1 + (ri-1)*M - copyto!(B, boffs, A, offs, M) - end - end - else - for i = 1:sd - ri = sd+1-i - for j=0:stride:(N-stride) - offs = j + 1 + (i-1)*M - boffs = j + 1 + (ri-1)*M - for k=0:(M-1) - B[boffs + k] = A[offs + k] - end - end - end + M == 0 && return A # nothing to reverse + + idx = Vector{Int}(undef, N) # temporary array for index calculations + + # compute sizes for half of reversed array + ntuple(k -> @inbounds(idx[k] = size(A,k)), Val{N}()) + @inbounds idx[dims[1]] = (idx[dims[1]] + 1) >> 1 + + last1 = ntuple(k -> lastindex(A,k)+1, Val{N}()) + for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(idx[k]), Val{N}())) + ntuple(k -> @inbounds(idx[k] = i[k]), Val{N}()) + ntuple(Val{M}()) do k + @inbounds j = dims[k] + @inbounds idx[j] = last1[j] - idx[j] end + iᵣ = CartesianIndex(ntuple(k -> @inbounds(idx[k]), Val{N}())) + @inbounds A[iᵣ], A[i] = A[i], A[iᵣ] end - return B + return A end +# fix ambiguity with array.jl: +_reverse!(A::AbstractVector, dim::Tuple{Int}) = _reverse!(A, first(dim)) + """ rotl90(A) diff --git a/base/bitarray.jl b/base/bitarray.jl index 4017cf2ccdc61..51eff74a3f50f 100644 --- a/base/bitarray.jl +++ b/base/bitarray.jl @@ -1163,8 +1163,8 @@ end # TODO some of this could be optimized -reverse(A::BitArray; dims::Integer) = _reverse_int(A, Int(dims)) -function _reverse_int(A::BitArray, d::Int) +_reverse(A::BitArray, d::Tuple{Integer}) = _reverse(A, d[1]) +function _reverse(A::BitArray, d::Int) nd = ndims(A) 1 ≤ d ≤ nd || throw(ArgumentError("dimension $d is not 1 ≤ $d ≤ $nd")) sd = size(A, d) @@ -1210,7 +1210,7 @@ function _reverse_int(A::BitArray, d::Int) return B end -function reverse!(B::BitVector) +function _reverse!(B::BitVector, ::Colon) # Basic idea: each chunk is divided into two blocks of size k = n % 64, and # h = 64 - k. Walk from either end (with indices i and j) reversing chunks # and separately ORing their two blocks into place. @@ -1264,9 +1264,6 @@ function reverse!(B::BitVector) return B end -reverse(v::BitVector) = reverse!(copy(v)) - - function (<<)(B::BitVector, i::UInt) n = length(B) i == 0 && return copy(B) diff --git a/base/range.jl b/base/range.jl index f241a8ac45003..8f1a1443b22fc 100644 --- a/base/range.jl +++ b/base/range.jl @@ -990,15 +990,15 @@ end Array{T,1}(r::AbstractRange{T}) where {T} = vcat(r) collect(r::AbstractRange) = vcat(r) -reverse(r::OrdinalRange) = (:)(last(r), -step(r), first(r)) -function reverse(r::StepRangeLen) +_reverse(r::OrdinalRange, ::Colon) = (:)(last(r), -step(r), first(r)) +function _reverse(r::StepRangeLen, ::Colon) # If `r` is empty, `length(r) - r.offset + 1 will be nonpositive hence # invalid. As `reverse(r)` is also empty, any offset would work so we keep # `r.offset` offset = isempty(r) ? r.offset : length(r)-r.offset+1 StepRangeLen(r.ref, -r.step, length(r), offset) end -reverse(r::LinRange{T}) where {T} = LinRange{T}(r.stop, r.start, length(r)) +_reverse(r::LinRange{T}, ::Colon) where {T} = LinRange{T}(r.stop, r.start, length(r)) ## sorting ## From 9d52c81be7f42149f447b1fb1df6915184f4736f Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 22:46:06 -0400 Subject: [PATCH 02/15] fix reverse for offsetarray --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 968bd61b3c35c..161a45f8e0504 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -108,7 +108,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} ntuple(k -> @inbounds(idx[k] = size(A,k)), Val{N}()) @inbounds idx[dims[1]] = (idx[dims[1]] + 1) >> 1 - last1 = ntuple(k -> lastindex(A,k)+1, Val{N}()) + last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(idx[k]), Val{N}())) ntuple(k -> @inbounds(idx[k] = i[k]), Val{N}()) ntuple(Val{M}()) do k From 3c19b73b6b29a048806f54c5286cc5e86530a941 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:02:25 -0400 Subject: [PATCH 03/15] bugfix for odd dimensions + test --- base/arraymath.jl | 8 +++++++- test/arrayops.jl | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 161a45f8e0504..1c844f73d3b37 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -106,7 +106,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} # compute sizes for half of reversed array ntuple(k -> @inbounds(idx[k] = size(A,k)), Val{N}()) - @inbounds idx[dims[1]] = (idx[dims[1]] + 1) >> 1 + @inbounds idx[dims[1]] = idx[dims[1]] >> 1 last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(idx[k]), Val{N}())) @@ -118,6 +118,12 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} iᵣ = CartesianIndex(ntuple(k -> @inbounds(idx[k]), Val{N}())) @inbounds A[iᵣ], A[i] = A[i], A[iᵣ] end + if M > 1 && isodd(size(A, dims[1])) + # middle slice for odd dimensions must be recursively flipped + mid = firstindex(A, dims[1]) + (size(A, dims[1]) >> 1) + midslice = CartesianIndices(ntuple(k -> k == dims[1] ? (mid:mid) : axes(A, k), Val{N}())) + _reverse!(@view(A[midslice]), dims[2:end]) + end return A end # fix ambiguity with array.jl: diff --git a/test/arrayops.jl b/test/arrayops.jl index 7cca191e3ff35..098399197c7f1 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -1555,6 +1555,27 @@ end @test reverse(["a" "b"; "c" "d"], dims = 2) == ["b" "a"; "d" "c"] end +@test "reverse multiple dims" begin + for A in (zeros(2,4), zeros(3,5)) + A[:] .= 1:length(A) # unique-ify elements + @test reverse(A) == reverse(reverse(A, dims=1), dims=2) == + reverse(A, dims=(1,2)) == reverse(A, dims=(2,1)) + @test_throws ArgumentError reverse(A, dims=(1,2,3)) + @test_throws ArgumentError reverse(A, dims=(1,2,2)) + end + for A in (zeros(2,4,6), zeros(3,5,7)) + A[:] .= 1:length(A) # unique-ify elements + @test reverse(A) == reverse(reverse(reverse(A, dims=1), dims=2), dims=3) == + reverse(reverse(A, dims=(1,2)), dims=3) == + reverse(reverse(A, dims=(2,3)), dims=1) == + reverse(reverse(A, dims=(1,3)), dims=2) == + reverse(A, dims=(1,2,3)) == reverse(A, dims=(3,2,1)) == reverse(A, dims=(2,1,3)) + @test reverse(A, dims=(1,2)) == reverse(reverse(A, dims=1), dims=2) + @test reverse(A, dims=(1,3)) == reverse(reverse(A, dims=1), dims=3) + @test reverse(A, dims=(2,3)) == reverse(reverse(A, dims=2), dims=3) + end +end + @testset "isdiag, istril, istriu" begin @test isdiag(3) @test istril(4) From cfff85442a34721fc7c029d18cf70b4a39f1ab94 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:05:20 -0400 Subject: [PATCH 04/15] typo --- test/arrayops.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index 098399197c7f1..be4a0583f0bb6 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -1555,7 +1555,7 @@ end @test reverse(["a" "b"; "c" "d"], dims = 2) == ["b" "a"; "d" "c"] end -@test "reverse multiple dims" begin +@testset "reverse multiple dims" begin for A in (zeros(2,4), zeros(3,5)) A[:] .= 1:length(A) # unique-ify elements @test reverse(A) == reverse(reverse(A, dims=1), dims=2) == From d18883d9b5d6cdde978c09382b3e83002bb83fee Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:17:59 -0400 Subject: [PATCH 05/15] fix bootstrap failure --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 1c844f73d3b37..4ae677ecb8f1c 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -122,7 +122,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} # middle slice for odd dimensions must be recursively flipped mid = firstindex(A, dims[1]) + (size(A, dims[1]) >> 1) midslice = CartesianIndices(ntuple(k -> k == dims[1] ? (mid:mid) : axes(A, k), Val{N}())) - _reverse!(@view(A[midslice]), dims[2:end]) + _reverse!(view(A, midslice), dims[2:end]) end return A end From ec224a5f53eb75395d981166b7523be7a767f8e5 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:19:54 -0400 Subject: [PATCH 06/15] explicit reverse! and dims=: tests --- test/arrayops.jl | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/arrayops.jl b/test/arrayops.jl index be4a0583f0bb6..f44a6999013ea 100644 --- a/test/arrayops.jl +++ b/test/arrayops.jl @@ -1558,21 +1558,22 @@ end @testset "reverse multiple dims" begin for A in (zeros(2,4), zeros(3,5)) A[:] .= 1:length(A) # unique-ify elements - @test reverse(A) == reverse(reverse(A, dims=1), dims=2) == + @test reverse(A) == reverse!(reverse(A, dims=1), dims=2) == reverse(A, dims=(1,2)) == reverse(A, dims=(2,1)) @test_throws ArgumentError reverse(A, dims=(1,2,3)) @test_throws ArgumentError reverse(A, dims=(1,2,2)) end for A in (zeros(2,4,6), zeros(3,5,7)) A[:] .= 1:length(A) # unique-ify elements - @test reverse(A) == reverse(reverse(reverse(A, dims=1), dims=2), dims=3) == - reverse(reverse(A, dims=(1,2)), dims=3) == - reverse(reverse(A, dims=(2,3)), dims=1) == - reverse(reverse(A, dims=(1,3)), dims=2) == + @test reverse(A) == reverse(A, dims=:) == reverse!(copy(A)) == reverse!(copy(A), dims=:) == + reverse!(reverse!(reverse(A, dims=1), dims=2), dims=3) == + reverse!(reverse(A, dims=(1,2)), dims=3) == + reverse!(reverse(A, dims=(2,3)), dims=1) == + reverse!(reverse(A, dims=(1,3)), dims=2) == reverse(A, dims=(1,2,3)) == reverse(A, dims=(3,2,1)) == reverse(A, dims=(2,1,3)) - @test reverse(A, dims=(1,2)) == reverse(reverse(A, dims=1), dims=2) - @test reverse(A, dims=(1,3)) == reverse(reverse(A, dims=1), dims=3) - @test reverse(A, dims=(2,3)) == reverse(reverse(A, dims=2), dims=3) + @test reverse(A, dims=(1,2)) == reverse!(reverse(A, dims=1), dims=2) + @test reverse(A, dims=(1,3)) == reverse!(reverse(A, dims=1), dims=3) + @test reverse(A, dims=(2,3)) == reverse!(reverse(A, dims=2), dims=3) end end From c3b33b8ca0a9b6ac42c63292bdb37d55063f3e7d Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:30:14 -0400 Subject: [PATCH 07/15] NEWS --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index 98c68b32b7a26..aa7122f59b5f3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -74,6 +74,9 @@ New library features Standard library changes ------------------------ * The `nextprod` function now accepts tuples and other array types for its first argument ([#35791]). +* The `reverse(A; dims)` function for multidimensional `A` can now reverse multiple dimensions at once + by passing a tuple for `dims`, and defaults to reversing all dimensions; there is also a multidimensional + in-place `reverse!(A; dims)` ([#37367]). * The function `isapprox(x,y)` now accepts the `norm` keyword argument also for numeric (i.e., non-array) arguments `x` and `y` ([#35883]). * `view`, `@view`, and `@views` now work on `AbstractString`s, returning a `SubString` when appropriate ([#35879]). * All `AbstractUnitRange{<:Integer}`s now work with `SubString`, `view`, `@view` and `@views` on strings ([#35879]). From b14eb94b8d155e7dfa5fdf0675a98ae49354ccdf Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Wed, 2 Sep 2020 23:34:15 -0400 Subject: [PATCH 08/15] more offsetarray tests --- test/offsetarray.jl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/offsetarray.jl b/test/offsetarray.jl index 9ce6a8a608cf9..cd5c5bc848ace 100644 --- a/test/offsetarray.jl +++ b/test/offsetarray.jl @@ -522,6 +522,12 @@ soa = OffsetArray([2,2,3], typemax(Int)-4) @test rotr90(A) == OffsetArray(rotr90(parent(A)), A.offsets[[2,1]]) @test reverse(A, dims=1) == OffsetArray(reverse(parent(A), dims=1), A.offsets) @test reverse(A, dims=2) == OffsetArray(reverse(parent(A), dims=2), A.offsets) +@test reverse(A) == reverse!(reverse(A, dims=1), dims=2) + +Aodd = OffsetArray(rand(3,5), (-3,5)) +@test reverse(Aodd, dims=1) == OffsetArray(reverse(parent(Aodd), dims=1), Aodd.offsets) +@test reverse(Aodd, dims=2) == OffsetArray(reverse(parent(Aodd), dims=2), Aodd.offsets) +@test reverse(Aodd) == reverse!(reverse(Aodd, dims=1), dims=2) @test A .+ 1 == OffsetArray(parent(A) .+ 1, A.offsets) @test 2*A == OffsetArray(2*parent(A), A.offsets) From ab23597085a004da06230a2d5e4f548f9ca3932c Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 07:56:40 -0400 Subject: [PATCH 09/15] compat annotation --- base/arraymath.jl | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/base/arraymath.jl b/base/arraymath.jl index 4ae677ecb8f1c..89fcb921d9b29 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -83,6 +83,9 @@ julia> reverse(b, dims=2) 4 3 2 1 ``` + +!!! compat "Julia 1.6" + Prior to Julia 1.6, only single-integer `dims` are supported in `reverse`. """ reverse(A::AbstractArray; dims=:) = _reverse(A, dims) _reverse(A, dims) = reverse!(copymutable(A); dims) @@ -91,6 +94,9 @@ _reverse(A, dims) = reverse!(copymutable(A); dims) reverse!(A; dims=:) Like [`reverse`](@ref), but operates in-place in `A`. + +!!! compat "Julia 1.6" + Multidimensional `reverse!` requires Julia 1.6. """ reverse!(A::AbstractArray; dims=:) = _reverse!(A, dims) _reverse!(A::AbstractArray{<:Any,N}, ::Colon) where {N} = _reverse!(A, ntuple(identity, Val{N}())) From 7d72c7573b62953357ef3ff804ab540214ef9355 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 10:25:51 -0400 Subject: [PATCH 10/15] further simplify and speed up reverse (and reverse! is now down to 0 allocations) --- base/arraymath.jl | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 89fcb921d9b29..de55cb4b45b0f 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -103,25 +103,20 @@ _reverse!(A::AbstractArray{<:Any,N}, ::Colon) where {N} = _reverse!(A, ntuple(id _reverse!(A, dim::Integer) = _reverse!(A, (Int(dim),)) _reverse!(A, dims::NTuple{M,Integer}) where {M} = _reverse!(A, Int.(dims)) function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} - if N < M || !allunique(dims) || !all(d -> 1 ≤ d ≤ N, dims) + dimrev = ntuple(k -> k in dims, Val{N}()) # boolean tuple indicating reversed dims + + if N < M || M != sum(dimrev) throw(ArgumentError("invalid dimensions $dims in reverse!")) end M == 0 && return A # nothing to reverse - idx = Vector{Int}(undef, N) # temporary array for index calculations - - # compute sizes for half of reversed array - ntuple(k -> @inbounds(idx[k] = size(A,k)), Val{N}()) - @inbounds idx[dims[1]] = idx[dims[1]] >> 1 + # swapping loop only needs to traverse ≈half of the array + halfsz = ntuple(k -> k == dims[1] ? size(A,k) >> 1 : size(A,k), Val{N}()) - last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) - for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(idx[k]), Val{N}())) - ntuple(k -> @inbounds(idx[k] = i[k]), Val{N}()) - ntuple(Val{M}()) do k - @inbounds j = dims[k] - @inbounds idx[j] = last1[j] - idx[j] - end - iᵣ = CartesianIndex(ntuple(k -> @inbounds(idx[k]), Val{N}())) + last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) # offset for reversed index + @inbounds for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(halfsz[k]), Val{N}())) + iₜ = Tuple(i) + iᵣ = CartesianIndex(ifelse.(dimrev, last1 .- iₜ, iₜ)) @inbounds A[iᵣ], A[i] = A[i], A[iᵣ] end if M > 1 && isodd(size(A, dims[1])) From f49cba8ab1edae108829a5371b6d62b83c9f6cd8 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 10:29:59 -0400 Subject: [PATCH 11/15] rm redundant inbounds --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index de55cb4b45b0f..e3d55fa91611a 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -114,7 +114,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} halfsz = ntuple(k -> k == dims[1] ? size(A,k) >> 1 : size(A,k), Val{N}()) last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) # offset for reversed index - @inbounds for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(halfsz[k]), Val{N}())) + for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(halfsz[k]), Val{N}())) iₜ = Tuple(i) iᵣ = CartesianIndex(ifelse.(dimrev, last1 .- iₜ, iₜ)) @inbounds A[iᵣ], A[i] = A[i], A[iᵣ] From d40867e5bb4de8f8a4e3149a5db9fa43ef785452 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 13:05:28 -0400 Subject: [PATCH 12/15] whoops extra space --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index e3d55fa91611a..421d738541d3d 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -78,7 +78,7 @@ julia> reverse(b, dims=2) 2 1 4 3 - julia> reverse(b) +julia> reverse(b) 2×2 Array{Int64,2}: 4 3 2 1 From 8fc5972b156bec8630c1c2b994eb33c351b28024 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 13:55:41 -0400 Subject: [PATCH 13/15] =?UTF-8?q?use=20=C3=B7=202=20instead=20of=20>>=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- base/arraymath.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 421d738541d3d..efbb0b7b1d54a 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -111,7 +111,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} M == 0 && return A # nothing to reverse # swapping loop only needs to traverse ≈half of the array - halfsz = ntuple(k -> k == dims[1] ? size(A,k) >> 1 : size(A,k), Val{N}()) + halfsz = ntuple(k -> k == dims[1] ? size(A,k) ÷ 2 : size(A,k), Val{N}()) last1 = ntuple(k -> lastindex(A,k)+firstindex(A,k), Val{N}()) # offset for reversed index for i in CartesianIndices(ntuple(k -> firstindex(A,k):firstindex(A,k)-1+@inbounds(halfsz[k]), Val{N}())) @@ -121,7 +121,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} end if M > 1 && isodd(size(A, dims[1])) # middle slice for odd dimensions must be recursively flipped - mid = firstindex(A, dims[1]) + (size(A, dims[1]) >> 1) + mid = firstindex(A, dims[1]) + (size(A, dims[1]) ÷ 2) midslice = CartesianIndices(ntuple(k -> k == dims[1] ? (mid:mid) : axes(A, k), Val{N}())) _reverse!(view(A, midslice), dims[2:end]) end From c44abc28f56b8da06bb7e61f15f0d01d095af645 Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 14:53:38 -0400 Subject: [PATCH 14/15] fix type output in doc --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index efbb0b7b1d54a..01ec976e256d5 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -79,7 +79,7 @@ julia> reverse(b, dims=2) 4 3 julia> reverse(b) -2×2 Array{Int64,2}: +2×2 Matrix{Int64}: 4 3 2 1 ``` From 2b30d1b7801bbefe8dd3ae837fc20025c1adbafe Mon Sep 17 00:00:00 2001 From: "Steven G. Johnson" Date: Thu, 3 Sep 2020 21:48:03 -0400 Subject: [PATCH 15/15] fix type instability in slice ranges --- base/arraymath.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/arraymath.jl b/base/arraymath.jl index 01ec976e256d5..e75e98bf9dd62 100644 --- a/base/arraymath.jl +++ b/base/arraymath.jl @@ -122,7 +122,7 @@ function _reverse!(A::AbstractArray{<:Any,N}, dims::NTuple{M,Int}) where {N,M} if M > 1 && isodd(size(A, dims[1])) # middle slice for odd dimensions must be recursively flipped mid = firstindex(A, dims[1]) + (size(A, dims[1]) ÷ 2) - midslice = CartesianIndices(ntuple(k -> k == dims[1] ? (mid:mid) : axes(A, k), Val{N}())) + midslice = CartesianIndices(ntuple(k -> k == dims[1] ? (mid:mid) : (firstindex(A,k):lastindex(A,k)), Val{N}())) _reverse!(view(A, midslice), dims[2:end]) end return A