Skip to content

Commit

Permalink
Various fixes for reinterpret(reshape, T, a) (#37858)
Browse files Browse the repository at this point in the history
  • Loading branch information
timholy authored Oct 4, 2020
1 parent 7746b91 commit 55a6dab
Show file tree
Hide file tree
Showing 5 changed files with 24 additions and 5 deletions.
11 changes: 7 additions & 4 deletions base/reinterpretarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ struct ReinterpretArray{T,N,S,A<:AbstractArray{S},IsReshaped} <: AbstractArray{T
writable = array_subpadding(S, T)
new{T, N, S, A, false}(a, readable, writable)
end
reinterpret(::Type{T}, a::AbstractArray{T}) where {T} = a

# With reshaping
function reinterpret(::typeof(reshape), ::Type{T}, a::A) where {T,S,A<:AbstractArray{S}}
Expand Down Expand Up @@ -74,6 +75,7 @@ struct ReinterpretArray{T,N,S,A<:AbstractArray{S},IsReshaped} <: AbstractArray{T
writable = array_subpadding(S, T)
new{T, N, S, A, true}(a, readable, writable)
end
reinterpret(::typeof(reshape), ::Type{T}, a::AbstractArray{T}) where {T} = a
end

ReshapedReinterpretArray{T,N,S,A<:AbstractArray{S}} = ReinterpretArray{T,N,S,A,true}
Expand All @@ -97,7 +99,7 @@ julia> A = [1 2; 3 4]
3 4
julia> reinterpret(reshape, Complex{Int}, A) # the result is a vector
2-element reinterpret(reshape, Complex{$Int}, ::Matrix{$Int}):
2-element reinterpret(reshape, Complex{$Int}, ::Matrix{$Int}) with eltype Complex{$Int}:
1 + 3im
2 + 4im
Expand All @@ -107,15 +109,16 @@ julia> a = [(1,2,3), (4,5,6)]
(4, 5, 6)
julia> reinterpret(reshape, Int, a) # the result is a matrix
3×2 reinterpret(reshape, $Int, ::Vector{Tuple{$Int, $Int, $Int}}):
3×2 reinterpret(reshape, $Int, ::Vector{Tuple{$Int, $Int, $Int}}) with eltype $Int:
1 4
2 5
3 6
```
"""
reinterpret(::typeof(reshape), T::Type, a::AbstractArray)

reinterpret(::Type{T}, a::ReinterpretArray) where {T} = reinterpret(T, a.parent)
reinterpret(::Type{T}, a::NonReshapedReinterpretArray) where {T} = reinterpret(T, a.parent)
reinterpret(::typeof(reshape), ::Type{T}, a::ReshapedReinterpretArray) where {T} = reinterpret(reshape, T, a.parent)

# Definition of StridedArray
StridedFastContiguousSubArray{T,N,A<:DenseArray} = FastContiguousSubArray{T,N,A}
Expand Down Expand Up @@ -316,7 +319,7 @@ end
# Convert to full indices here, to avoid needing multiple conversions in
# the loop in _getindex_ra
inds = _to_subscript_indices(a, i)
_getindex_ra(a, inds[1], tail(inds))
isempty(inds) ? _getindex_ra(a, 1, ()) : _getindex_ra(a, inds[1], tail(inds))
end

@inline @propagate_inbounds function getindex(a::ReshapedReinterpretArray{T,N,S}, ind::SCartesianIndex2) where {T,N,S}
Expand Down
2 changes: 1 addition & 1 deletion base/reshapedarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ end
_reshape(v::ReshapedArray{<:Any,1}, dims::Dims{1}) = _reshape(v.parent, dims)
_reshape(R::ReshapedArray, dims::Dims) = _reshape(R.parent, dims)

function __reshape(p::Tuple{AbstractArray,IndexCartesian}, dims::Dims)
function __reshape(p::Tuple{AbstractArray,IndexStyle}, dims::Dims)
parent = p[1]
strds = front(size_to_strides(map(length, axes(parent))..., 1))
strds1 = map(s->max(1,Int(s)), strds) # for resizing empty arrays
Expand Down
1 change: 1 addition & 0 deletions base/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2572,6 +2572,7 @@ function showarg(io::IO, r::ReshapedReinterpretArray{T}, toplevel) where {T}
print(io, "reinterpret(reshape, ", T, ", ")
showarg(io, parent(r), false)
print(io, ')')
toplevel && print(io, " with eltype ", eltype(r))
end

# printing iterators from Base.Iterators
Expand Down
13 changes: 13 additions & 0 deletions test/reinterpretarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,19 @@ B[] = Int32(5)
@test Brs[] === Int32(5)
@test A[] === UInt32(5)

a = [(1.0,2.0)]
af = @inferred(reinterpret(reshape, Float64, a))
anew = @inferred(reinterpret(reshape, Tuple{Float64,Float64}, vec(af)))
@test anew[1] == a[1]
@test ndims(anew) == 0

# re-reinterpret
a0 = reshape([0x22, 0x44, 0x88, 0xf0, 0x01, 0x02, 0x03, 0x04], 4, 2)
a = reinterpret(reshape, NTuple{4,UInt8}, a0)
@test a == [(0x22, 0x44, 0x88, 0xf0), (0x01, 0x02, 0x03, 0x04)]
@test reinterpret(UInt8, a) == [0x22, 0x44, 0x88, 0xf0, 0x01, 0x02, 0x03, 0x04]
@test reinterpret(reshape, UInt8, a) === a0

# reductions
a = [(1,2,3), (4,5,6)]
ars = reinterpret(reshape, Int, a)
Expand Down
2 changes: 2 additions & 0 deletions test/show.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,8 @@ end
@test summary(r) == "4×2 reshape(view(::Array{Int16, 3}, :, 3, 2:5), 4, 2) with eltype Int16"
p = PermutedDimsArray(r, (2, 1))
@test summary(p) == "2×4 PermutedDimsArray(reshape(view(::Array{Int16, 3}, :, 3, 2:5), 4, 2), (2, 1)) with eltype Int16"
p = reinterpret(reshape, Tuple{Float32,Float32}, [1.0f0 3.0f0; 2.0f0 4.0f0])
@test summary(p) == "2-element reinterpret(reshape, Tuple{Float32, Float32}, ::Matrix{Float32}) with eltype Tuple{Float32, Float32}"
end

@testset "Methods" begin
Expand Down

4 comments on commit 55a6dab

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.