From e8b9ede3e5b8611e8ac181806272ec7af30f98f5 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sat, 29 Aug 2020 06:38:59 -0500 Subject: [PATCH 1/2] Improve the benchmarks --- test/benchmarks.jl | 85 ++++++++++++++++++++++++++++++---------------- 1 file changed, 56 insertions(+), 29 deletions(-) diff --git a/test/benchmarks.jl b/test/benchmarks.jl index 2047b75..a6d6042 100644 --- a/test/benchmarks.jl +++ b/test/benchmarks.jl @@ -54,13 +54,15 @@ end function test_getindex(f, ar, cv, n) t_ar = Array{Float64}(undef, n) t_cv = Array{Float64}(undef, n) + # Store the results to prevent the compiler from eliding the call f_ar = Ref(f(ar)) f_cv = Ref(f(cv)) + @test f_ar[] ≈ f_cv[] # but this also gives us a chance to test correctness for i = 1:n t_ar[i] = (tstart = time(); f_ar[] = f(ar); time()-tstart) t_cv[i] = (tstart = time(); f_cv[] = f(cv); time()-tstart) end - median(t_ar), median(t_cv), f_ar + median(t_ar), median(t_cv) end function test_setindex(f, ar, cv, n) t_ar = Array{Float64}(undef, n) @@ -72,45 +74,70 @@ function test_setindex(f, ar, cv, n) median(t_ar), median(t_cv) end -ssz = (1000,1000) -c = rand(RGB{Float64}, ssz...) -a = copy(reinterpretc(Float64, c)) -vchan = channelview(c) -vcol = colorview(RGB, a) cc_getindex_funcs = (mysum_elt_boundscheck, mysum_index_boundscheck, mysum_elt_inbounds, mysum_index_inbounds_simd) cc_setindex_funcs = (myfill1!, myfill2!) -chanvtol = Dict(mysum_index_inbounds_simd => 20, # @simd doesn't work for ChannelView :( - mysum_elt_boundscheck => 20, - myfill1! => 20, # crappy setindex! performance - myfill2! => 20) -chanvdefault = 10 -colvtol = Dict(mysum_elt_boundscheck=>5, - mysum_index_boundscheck=>5) + +# Performance tolerances +isfast = VERSION >= v"1.6.0-DEV.1083" +chanvtol = Dict(mysum_index_inbounds_simd => isfast ? 3 : 20, + mysum_elt_boundscheck => isfast ? 3 : 20, + myfill1! => 20, + myfill2! => isfast ? 3 : 20) +chanvdefault = isfast ? 3 : 10 +colvtol = Dict(mysum_elt_boundscheck=>isfast ? 3 : 5, + mysum_index_boundscheck=>isfast ? 3 : 5) colvdefault = 3 +ssz = (1000,300) + @info "Benchmark tests are warnings for now" # @testset "benchmarks" begin -for (suite, testf) in ((cc_getindex_funcs, test_getindex), - (cc_setindex_funcs, test_setindex)) - for f in suite - # Channelview - t_ar, t_cv = testf(f, a, vchan, 10^2) - tol = haskey(chanvtol, f) ? chanvtol[f] : chanvdefault - if t_cv >= tol*t_ar - @warn "ChannelView: failed on $f, time ratio $(t_cv/t_ar), tol $tol" - end - # @test t_cv < tol*t_ar - # ColorView - t_ar, t_cv = testf(f, c, vcol, 10^2) - tol = haskey(colvtol, f) ? colvtol[f] : colvdefault - if t_cv >= tol*t_ar - @warn "ColorView: failed on $f, time ratio $(t_cv/t_ar), tol $tol" +for T in (Float32, Float64) + c = rand(RGB{T}, ssz...) + a = copy(reinterpretc(T, c)) + vchan = channelview(c) + vcol = colorview(RGB, a) + + # view versions + rview = 2:ssz[1]-1 + csub = view(c, rview, :) + asub = view(a, :, rview, :) + vchansub = channelview(csub) + vcolsub = colorview(RGB, asub) + + for (suite, testf) in ((cc_getindex_funcs, test_getindex), + (cc_setindex_funcs, test_setindex)) + for f in suite + # channelview + t_ar, t_cv = testf(f, a, vchan, 30) + tol = haskey(chanvtol, f) ? chanvtol[f] : chanvdefault + if t_cv >= tol*t_ar + @warn "channelview1: failed on $f with eltype $T, time ratio $(t_cv/t_ar), tol $tol" + end + + t_ar, t_cv = testf(f, asub, vchansub, 30) + tol = haskey(chanvtol, f) ? chanvtol[f] : chanvdefault + if t_cv >= tol*t_ar + @warn "channelview2: failed on $f with eltype $T, time ratio $(t_cv/t_ar), tol $tol" + end + + # colorview + t_ar, t_cv = testf(f, c, vcol, 30) + tol = haskey(colvtol, f) ? colvtol[f] : colvdefault + if t_cv >= tol*t_ar + @warn "colorview1: failed on $f with eltype $T, time ratio $(t_cv/t_ar), tol $tol" + end + + t_ar, t_cv = testf(f, csub, vcolsub, 30) + tol = haskey(colvtol, f) ? colvtol[f] : colvdefault + if t_cv >= tol*t_ar + @warn "colorview2: failed on $f with eltype $T, time ratio $(t_cv/t_ar), tol $tol" + end end - # @test t_cv < tol*t_ar end end # end From 9ab90892c1c2c7957d8259fa0d89777c8760e997 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Sun, 13 Sep 2020 06:17:51 -0500 Subject: [PATCH 2/2] Use `reinterpret(reshape, T, A)` when supported --- src/ImageCore.jl | 6 ++- src/colorchannels.jl | 43 ++++++++++++----- src/convert_reinterpret.jl | 96 ++++++++++++++++++++----------------- test/colorchannels.jl | 2 +- test/convert_reinterpret.jl | 4 +- test/show.jl | 18 ++++--- 6 files changed, 103 insertions(+), 66 deletions(-) diff --git a/src/ImageCore.jl b/src/ImageCore.jl index 2225092..863303d 100644 --- a/src/ImageCore.jl +++ b/src/ImageCore.jl @@ -43,7 +43,11 @@ ColorA{N,C,T} = ColorAlpha{C,T,N} const NonparametricColors = Union{RGB24,ARGB32,Gray24,AGray32} Color1Array{C<:Color1,N} = AbstractArray{C,N} # Type that arises from reshape(reinterpret(To, A), sz): -const RRArray{To,From,N,M,P} = Base.ReshapedArray{To,N,Base.ReinterpretArray{To,M,From,P}} +if VERSION >= v"1.6.0-DEV.1083" + const RRArray{To,From,M,P} = Base.ReinterpretArray{To,M,From,P,true} +else + const RRArray{To,From,N,M,P} = Base.ReshapedArray{To,N,Base.ReinterpretArray{To,M,From,P}} +end const RGArray = Union{Base.ReinterpretArray{<:AbstractGray,M,<:Number,P}, Base.ReinterpretArray{<:Number,M,<:AbstractGray,P}} where {M,P} # delibrately not export these constants to enable extensibility for downstream packages diff --git a/src/colorchannels.jl b/src/colorchannels.jl index e11a6d2..c530196 100644 --- a/src/colorchannels.jl +++ b/src/colorchannels.jl @@ -28,8 +28,13 @@ dimorder(::Type{<:AlphaColor{<:Color3,T,N}}) where {T,N} = ColorChanPerm((4, 1, const ColorChanPermIndexType{NC} = Tuple{<:ColorChanPerm,Vararg{<:Base.Slice,NC}} const ColorChanPermSubArray{T,N,P,I<:ColorChanPermIndexType,L} = SubArray{T,N,P,I,L} -const RRPermArray{To,From,N,M,P<:ColorChanPermSubArray} = - RRArray{To,From,N,M,P} +if VERSION >= v"1.6.0-DEV.1083" + const RRPermArray{To,From,M,P<:ColorChanPermSubArray} = + RRArray{To,From,M,P} +else + const RRPermArray{To,From,N,M,P<:ColorChanPermSubArray} = + RRArray{To,From,N,M,P} +end # This type exists solely to set multiple values in the color channel axis struct NVector{T,N} <: AbstractVector{T} @@ -62,8 +67,13 @@ A = channelview(img) # a 3×10×10 array See also: [`colorview`](@ref) """ channelview(A::AbstractArray{T}) where {T<:Number} = A -channelview(A::RRPermArray{<:Colorant,<:Number}) = parent(parent(parent(A))) -channelview(A::RRArray{<:Colorant,<:Number}) = parent(parent(A)) +if VERSION >= v"1.6.0-DEV.1083" + channelview(A::RRArray{<:Colorant,<:Number}) = parent(A) + channelview(A::RRPermArray{<:Colorant,<:Number}) = parent(parent(A)) +else + channelview(A::RRArray{<:Colorant,<:Number}) = parent(parent(A)) + channelview(A::RRPermArray{<:Colorant,<:Number}) = parent(parent(parent(A))) +end channelview(A::Base.ReinterpretArray{<:AbstractGray,M,<:Number}) where M = parent(A) channelview(A::AbstractArray{RGB{T}}) where {T} = reinterpretc(T, A) function channelview(A::AbstractArray{C}) where {C<:AbstractRGB} @@ -108,14 +118,25 @@ _ccolorview(::Type{C}, A::RRPermArray{T,C}) where {C<:Colorant,T<:Number} = parent(parent(parent(A))) _ccolorview(::Type{C}, A::RRArray{T,C}) where {C<:Colorant,T<:Number} = parent(parent(A)) -_ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:AbstractGray,T<:Number,M} = - parent(A) -_ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:RGB,T<:Number,M} = - reshape(parent(A), Base.tail(axes(parent(A)))) +if VERSION >= v"1.6.0-DEV.1083" + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C,AA,false}) where {C<:RGB,T<:Number,M,AA} = + reshape(parent(A), Base.tail(axes(parent(A)))) + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C,AA,true}) where {C<:RGB,T<:Number,M,AA} = + parent(A) + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C,AA,false}) where {C<:Color,T<:Number,M,AA} = + reshape(parent(A), Base.tail(axes(parent(A)))) + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C,AA,true}) where {C<:Color,T<:Number,M,AA} = + parent(A) +else + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:AbstractGray,T<:Number,M} = + parent(A) + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:RGB,T<:Number,M} = + reshape(parent(A), Base.tail(axes(parent(A)))) + _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:Color,T<:Number,M} = + reshape(parent(A), Base.tail(axes(parent(A)))) + end _ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:AbstractRGB,T<:Number,M} = _colorview_reorder(C, A) -_ccolorview(::Type{C}, A::Base.ReinterpretArray{T,M,C}) where {C<:Color,T<:Number,M} = - reshape(parent(A), Base.tail(axes(parent(A)))) _ccolorview(::Type{C}, A::AbstractArray{T}) where {C<:Colorant,T<:Number} = __ccolorview(C, A) # necessary to avoid ambiguities from dispatch on eltype __ccolorview(::Type{C}, A::AbstractArray{T}) where {T<:Number,C<:RGB{T}} = reinterpretc(C, A) @@ -178,7 +199,7 @@ Create a function that is equivalent to `(As...) -> colorview(C, Ax...)`. ```jldoctest; setup = :(using ImageCore) julia> ones(Float32, 2, 2) |> colorview(Gray) -2×2 reinterpret(Gray{Float32}, ::$(Array{Float32,2})): +2×2 reinterpret($(VERSION >= v"1.6.0-DEV.1083" ? "reshape, " : "")Gray{Float32}, ::$(Array{Float32,2}))$(VERSION >= v"1.6.0-DEV.1083" ? " with eltype Gray{Float32}" : ""): Gray{Float32}(1.0) Gray{Float32}(1.0) Gray{Float32}(1.0) Gray{Float32}(1.0) ``` diff --git a/src/convert_reinterpret.jl b/src/convert_reinterpret.jl index 4f11b3f..569bf8e 100644 --- a/src/convert_reinterpret.jl +++ b/src/convert_reinterpret.jl @@ -2,61 +2,67 @@ @pure samesize(::Type{T}, ::Type{S}) where {T,S} = sizeof(T) == sizeof(S) -## Color->Color -# function reinterpretc(::Type{CV1}, a::Array{CV2,1}) where {CV1<:Colorant,CV2<:Colorant} -# CV = ccolor(CV1, CV2) -# l = (length(a)*sizeof(CV2))÷sizeof(CV1) -# l*sizeof(CV1) == length(a)*sizeof(CV2) || throw(ArgumentError("sizes are incommensurate")) -# reshape(reinterpret(CV, a), (l,)) -# end -function reinterpretc(::Type{CV1}, a::AbstractArray{CV2}) where {CV1<:Colorant,CV2<:Colorant} - CV = ccolor(CV1, CV2) - if samesize(CV, CV2) - return reshape(reinterpret(CV, a), size(a)) +if VERSION >= v"1.6.0-DEV.1083" + reinterpretc(::Type{T}, a::AbstractArray) where T = reinterpret(reshape, ccolor_number(T, eltype(a)), a) + reinterpretc(::Type{T}, a::Base.ReinterpretArray{S,N,T,AA,true}) where {T,S,N,AA<:AbstractArray{T}} = parent(a) +else + # Color->Color + function reinterpretc(::Type{CV1}, a::Array{CV2,1}) where {CV1<:Colorant,CV2<:Colorant} + CV = ccolor(CV1, CV2) + l = (length(a)*sizeof(CV2))÷sizeof(CV1) + l*sizeof(CV1) == length(a)*sizeof(CV2) || throw(ArgumentError("sizes are incommensurate")) + reshape(reinterpret(CV, a), (l,)) end - throw(ArgumentError("result shape not specified")) -end - -## Color->T -function reinterpretc(::Type{T}, a::AbstractArray{CV}) where {T<:Number,CV<:Colorant} - if samesize(T, CV) - return reinterpret(T, a) + function reinterpretc(::Type{CV1}, a::AbstractArray{CV2}) where {CV1<:Colorant,CV2<:Colorant} + CV = ccolor(CV1, CV2) + if samesize(CV, CV2) + return reshape(reinterpret(CV, a), size(a)) + end + throw(ArgumentError("result shape not specified")) end - axs = axes(a) - if sizeof(CV) == sizeof(T)*_len(CV) - return reinterpret(T, reshape(a, Base.OneTo(1), axs...)) + + # Color->T + function reinterpretc(::Type{T}, a::AbstractArray{CV}) where {T<:Number,CV<:Colorant} + if samesize(T, CV) + return reinterpret(T, a) + end + axs = axes(a) + if sizeof(CV) == sizeof(T)*_len(CV) + return reinterpret(T, reshape(a, Base.OneTo(1), axs...)) + end + throw(ArgumentError("result shape not specified")) end - throw(ArgumentError("result shape not specified")) -end -reinterpretc(::Type{T}, a::AbstractArray{CV,0}) where {T<:Number,CV<:Colorant} = - reinterpret(T, reshape(a, 1)) + reinterpretc(::Type{T}, a::AbstractArray{CV,0}) where {T<:Number,CV<:Colorant} = + reinterpret(T, reshape(a, 1)) -_len(::Type{C}) where {C} = _len(C, eltype(C)) -_len(::Type{C}, ::Type{Any}) where {C} = error("indeterminate type") -_len(::Type{C}, ::Type{T}) where {C,T} = sizeof(C) ÷ sizeof(T) + _len(::Type{C}) where {C} = _len(C, eltype(C)) + _len(::Type{C}, ::Type{Any}) where {C} = error("indeterminate type") + _len(::Type{C}, ::Type{T}) where {C,T} = sizeof(C) ÷ sizeof(T) -## T->Color -# We have to distinguish two forms of call: -# form 1: reinterpretc(RGB{N0f8}, img) -# form 2: reinterpretc(RGB, img) -function reinterpretc(CV::Type{<:Colorant}, a::AbstractArray{T}) where T<:Number # {CV<:Colorant,T<:Number} - @noinline throwdm(C::Type, ind1) = - throw(DimensionMismatch("indices $ind1 are not consistent with color type $C")) - CVT = ccolor_number(CV, T) - if samesize(CVT, T) - return reinterpret(CVT, a) - end - axs = axes(a) - if axs[1] == Base.OneTo(sizeof(CVT) ÷ sizeof(eltype(CVT))) - return reshape(reinterpret(CVT, a), tail(axs)) + ## T->Color + # We have to distinguish two forms of call: + # form 1: reinterpretc(RGB{N0f8}, img) + # form 2: reinterpretc(RGB, img) + function reinterpretc(CV::Type{<:Colorant}, a::AbstractArray{T}) where T<:Number # {CV<:Colorant,T<:Number} + @noinline throwdm(C::Type, ind1) = + throw(DimensionMismatch("indices $ind1 are not consistent with color type $C")) + CVT = ccolor_number(CV, T) + if samesize(CVT, T) + return reinterpret(CVT, a) + end + axs = axes(a) + if axs[1] == Base.OneTo(sizeof(CVT) ÷ sizeof(eltype(CVT))) + return reshape(reinterpret(CVT, a), tail(axs)) + end + throwdm(CV, axs[1]) end - throwdm(CV, axs[1]) end # ccolor_number converts form 2 calls to form 1 calls -ccolor_number(::Type{CV}, ::Type{T}) where {CV<:Colorant,T<:Number} = +ccolor_number(::Type{T}, ::Any) where T<:Number = T +ccolor_number(::Type{CV}, ::Type{T}) where {CV<:Colorant,T} = ccolor_number(CV, eltype(CV), T) -ccolor_number(::Type{CV}, ::Type{CVT}, ::Type{T}) where {CV,CVT<:Number,T} = CV # form 1 +ccolor_number(::Type{CV}, ::Type{CVT}, ::Type{T}) where {CV,CVT<:Number,T} = CV # form 1 ccolor_number(::Type{CV}, ::Type{Any}, ::Type{T}) where {CV<:Colorant,T} = CV{T} # form 2 # for docstrings in the operations below diff --git a/test/colorchannels.jl b/test/colorchannels.jl index 2566ce9..aea99bc 100644 --- a/test/colorchannels.jl +++ b/test/colorchannels.jl @@ -407,7 +407,7 @@ end @test @inferred(axes(v)) == (IdentityUnitRange(-1:1), IdentityUnitRange(-2:2)) @test @inferred(v[0,0]) === RGB(a[1,0,0], a[2,0,0], a[3,0,0]) a = OffsetArray(rand(3, 3, 5), 0:2, -1:1, -2:2) - @test_throws DimensionMismatch colorview(RGB, a) + @test_throws (VERSION >= v"1.6.0-DEV.1083" ? ArgumentError : DimensionMismatch) colorview(RGB, a) end end diff --git a/test/convert_reinterpret.jl b/test/convert_reinterpret.jl index 36a08fd..0b96b06 100644 --- a/test/convert_reinterpret.jl +++ b/test/convert_reinterpret.jl @@ -129,9 +129,9 @@ using Test, Random # indeterminate type tests a = Array{RGB{AbstractFloat}}(undef, 3) - @test_throws ErrorException reinterpretc(Float64, a) + @test_throws Union{ArgumentError,ErrorException} reinterpretc(Float64, a) a = Vector{RGB}(undef, 3) - @test_throws ErrorException reinterpretc(Float64, a) + @test_throws Union{ArgumentError,ErrorException} reinterpretc(Float64, a) # Invalid conversions a = rand(UInt8, 4,5) diff --git a/test/show.jl b/test/show.jl index 41da7d1..f5bcba8 100644 --- a/test/show.jl +++ b/test/show.jl @@ -6,6 +6,9 @@ else sumsz(img) = "" end +const rrstr = VERSION >= v"1.6.0-DEV.1083" ? "reshape, " : "" +rrdim(n) = VERSION >= v"1.6.0-DEV.1083" ? n-1 : n + # N0f8 is shown as either N0f8 or Normed{UInt8, 8} # RGB is shown as ColorTypes.RGB or RGB function typestring(::Type{T}) where T @@ -22,13 +25,15 @@ RGB_str = typestring(RGB) v = view(rgb32, 2:3, :) @test summary(v) == "2×5 view(::Array{RGB{Float32},2}, 2:3, :) with eltype $(RGB_str){Float32}" a = channelview(rgb32) - @test summary(a) == "3×3×5 reinterpret(Float32, ::Array{RGB{Float32},3})" + @test summary(a) == (VERSION >= v"1.6.0-DEV.1083" ? "3×3×5 reinterpret(reshape, Float32, ::Array{RGB{Float32},2}) with eltype Float32" : + "3×3×5 reinterpret(Float32, ::Array{RGB{Float32},3})") num64 = rand(3,5) b = colorview(RGB, num64) - @test summary(b) == "5-element reshape(reinterpret(RGB{Float64}, ::$(typeof(num64))), 5) with eltype $(RGB_str){Float64}" + @test summary(b) == (VERSION >= v"1.6.0-DEV.1083" ? "5-element reinterpret(reshape, RGB{Float64}, ::$(typeof(num64))) with eltype $(RGB_str){Float64}" : + "5-element reshape(reinterpret(RGB{Float64}, ::$(typeof(num64))), 5) with eltype $(RGB_str){Float64}") rgb8 = rand(RGB{N0f8}, 3, 5) c = rawview(channelview(rgb8)) - @test summary(c) == "3×3×5 rawview(reinterpret(N0f8, ::Array{RGB{N0f8},3})) with eltype UInt8" + @test summary(c) == "3×3×5 rawview(reinterpret($(rrstr)N0f8, ::Array{RGB{N0f8},$(rrdim(3))})) with eltype UInt8" @test summary(rgb8) == "3×5 Array{RGB{N0f8},2} with eltype $(RGB_str){$(N0f8_str)}" rand8 = rand(UInt8, 3, 5) d = normedview(PermutedDimsArray(rand8, (2,1))) @@ -39,14 +44,15 @@ RGB_str = typestring(RGB) f = PermutedDimsArray(normedview(N0f16, rand16), (2,1)) @test summary(f) == "5×3 PermutedDimsArray(reinterpret(N0f16, ::$(typeof(rand16))), (2, 1)) with eltype $(N0f16_str)" g = channelview(rgb8) - @test summary(g) == "3×3×5 reinterpret(N0f8, ::Array{RGB{N0f8},3})" + etstr = VERSION >= v"1.6.0-DEV.1083" ? " with eltype N0f8" : "" + @test summary(g) == "3×3×5 reinterpret($(rrstr)N0f8, ::Array{RGB{N0f8},$(rrdim(3))})$etstr" h = OffsetArray(rgb8, -1:1, -2:2) @test summary(h) == "$(sumsz(h))OffsetArray(::Array{RGB{N0f8},2}, -1:1, -2:2) with eltype $(RGB_str){$(N0f8_str)} with indices -1:1×-2:2" i = channelview(h) - @test summary(i) == "$(sumsz(i))reinterpret(N0f8, OffsetArray(::Array{RGB{N0f8},3}, 1:1, -1:1, -2:2)) with indices 1:3×-1:1×-2:2" + @test summary(i) == "$(sumsz(i))reinterpret($(rrstr)N0f8, OffsetArray(::Array{RGB{N0f8},$(rrdim(3))}, 1:1, -1:1, -2:2)) with indices 1:3×-1:1×-2:2" c = channelview(rand(RGB{N0f8}, 2)) o = OffsetArray(c, -1:1, 0:1) - @test summary(o) == "$(sumsz(o))OffsetArray(reinterpret(N0f8, ::Array{RGB{N0f8},2}), -1:1, 0:1) with eltype $(N0f8_str) with indices -1:1×0:1" + @test summary(o) == "$(sumsz(o))OffsetArray(reinterpret($(rrstr)N0f8, ::Array{RGB{N0f8},$(rrdim(2))}), -1:1, 0:1) with eltype $(N0f8_str) with indices -1:1×0:1" # Issue #45 a = collect(tuple()) @test summary(a) == "0-element $(typeof(a))"