Use parent for similar(::PermutedDimsArray) #35304

mcabbott · 2020-03-30T08:02:25Z

This ensures that similar matches the underlying storage of a PermutedDimsArray wrapper, as happens e.g. for Transpose, views:

julia> using SparseArrays

julia> similar(transpose(sparse(ones(3,3))))
3×3 SparseMatrixCSC{Float64,Int64} with 0 stored entries

julia> similar(view(sparse(ones(3,3)), 1,:), 4,4)
4×4 SparseMatrixCSC{Float64,Int64} with 0 stored entries

julia> similar(PermutedDimsArray(sparse(ones(3,3)), (2,1)))
3×3 Array{Float64,2}:
 2.19322e-314  2.19306e-314  2.19306e-314
 0.0           0.0           0.0
 2.19307e-314  2.19307e-314  2.19307e-314

I'm not sure how to add a test for this, and don't see any in test/subarray.jl for views either. Can I load SparseArrays in the tests for Base?

Fixes JuliaGPU/CuArrays.jl#658

timholy

This is the right thing to do. Re testing, in #35298 I moved some useful array types into test/testhelpers; if that PR gets merged you could conceivably use TSlow for testing, since it has a custom similar method. It has the advantage of being guaranteed not to have any specialized methods written for it other than what's in the test file; the same cannot be said of sparse array types which tend to accumulate an ever-growing number of specializations.

timholy · 2020-03-30T09:47:09Z

base/permuteddimsarray.jl

 Base.axes(A::PermutedDimsArray{T,N,perm}) where {T,N,perm} = genperm(axes(parent(A)), perm)

+Base.similar(A::PermutedDimsArray, T::Type, dims::Base.Dims) = similar(parent(A), T, dims)


Since Transpose and Adjoint are for linear algebra which we currently support only for arrays with indexing starting with 1, it's OK for those to only support Dims. But PermutedDimsArray supports more general array types, so we have to consider the case where similar will receive axes rather than dims input. From

julia/base/abstractarray.jl

Lines 624 to 645 in 12acd69

similar(a::AbstractArray{T}) where {T} = similar(a, T)

similar(a::AbstractArray, ::Type{T}) where {T} = similar(a, T, to_shape(axes(a)))

similar(a::AbstractArray{T}, dims::Tuple) where {T} = similar(a, T, to_shape(dims))

similar(a::AbstractArray{T}, dims::DimOrInd...) where {T} = similar(a, T, to_shape(dims))

similar(a::AbstractArray, ::Type{T}, dims::DimOrInd...) where {T} = similar(a, T, to_shape(dims))

# Similar supports specifying dims as either Integers or AbstractUnitRanges or any mixed combination

# thereof. Ideally, we'd just convert Integers to OneTos and then call a canonical method with the axes,

# but we don't want to require all AbstractArray subtypes to dispatch on Base.OneTo. So instead we

# define this method to convert supported axes to Ints, with the expectation that an offset array

# package will define a method with dims::Tuple{Union{Integer, UnitRange}, Vararg{Union{Integer, UnitRange}}}

similar(a::AbstractArray, ::Type{T}, dims::Tuple{Union{Integer, OneTo}, Vararg{Union{Integer, OneTo}}}) where {T} = similar(a, T, to_shape(dims))

# similar creates an Array by default

similar(a::AbstractArray, ::Type{T}, dims::Dims{N}) where {T,N} = Array{T,N}(undef, dims)

to_shape(::Tuple{}) = ()

to_shape(dims::Dims) = dims

to_shape(dims::DimsOrInds) = map(to_shape, dims)::DimsOrInds

# each dimension

to_shape(i::Int) = i

to_shape(i::Integer) = Int(i)

to_shape(r::OneTo) = Int(last(r))

to_shape(r::AbstractUnitRange) = r

to avoid ambiguities it looks like we might need

similar(a::PermutedDimsArray, ::Type{T}, dims::Tuple{Union{Integer, OneTo}, Vararg{Union{Integer, OneTo}}}) where T similar(a::PermutedDimsArray, ::Type{T}, dims::Dims{N}) where {T,N} similar(a::PermutedDimsArray, ::Type{T}, axs::Tuple{Union{Integer, AbstractUnitRange}, Vararg{Union{Integer, AbstractUnitRange}}}) where T

OK, thanks for looking up what's needed, I will add these methods.

Here's what happens if I do add those methods:

using SparseArrays, OffsetArrays sp = sparse(ones(3,3)) similar(transpose(sp), 3,3) similar(transpose(sp), 0:2,0:2) similar(PermutedDimsArray(sp, (2,1)), 3,3) # isa Array similar(PermutedDimsArray(sp, (2,1)), 0:2,0:2) # OffsetArray(Array) # initial PR: Base.similar(A::PermutedDimsArray, T::Type, dims::Base.Dims) = similar(parent(A), T, dims) similar(PermutedDimsArray(sp, (2,1)), 3,3) # isa SparseMatrixCSC similar(PermutedDimsArray(sp, (2,1)), 0:2,0:2) # OffsetArray(SparseMatrixCSC) # add extra methods: Base.similar(A::PermutedDimsArray, ::Type{T}, dims::Tuple{Union{Integer, Base.OneTo}, Vararg{Union{Integer, Base.OneTo}}}) where T = similar(parent(A), T, dims) Base.similar(A::PermutedDimsArray, ::Type{T}, axs::Tuple{Union{Integer, AbstractUnitRange}, Vararg{Union{Integer, AbstractUnitRange}}}) where T = similar(parent(A), T, axs) similar(PermutedDimsArray(sp, (2,1)), 3,3) # isa SparseMatrixCSC similar(PermutedDimsArray(sp, (2,1)), 0:2,0:2) # is ambiguous. Candidates: # similar(A::PermutedDimsArray, ::Type{T}, axs::Tuple{Union{Integer, AbstractUnitRange},Vararg{Union{Integer, AbstractUnitRange},N} where N}) where T in Main at REPL[13]:1 # similar(A::AbstractArray, ::Type{T}, inds::Tuple{Union{Colon, Integer, Base.IdentityUnitRange, Base.OneTo, UnitRange, OffsetArrays.IdOffsetRange},Vararg{Union{Colon, Integer, Base.IdentityUnitRange, Base.OneTo, UnitRange, OffsetArrays.IdOffsetRange},N} where N}) where T in OffsetArrays at /Users/me/.julia/packages/OffsetArrays/WmVaB/src/OffsetArrays.jl:102 # Possible fix, define # similar(::PermutedDimsArray, ::Type{T}, ::Tuple{Union{Integer, Base.IdentityUnitRange, Base.OneTo, UnitRange, OffsetArrays.IdOffsetRange},Vararg{Union{Integer, Base.IdentityUnitRange, Base.OneTo, UnitRange, OffsetArrays.IdOffsetRange},N} where N}) where T

We need to solve this at the level of OffsetArrays, presumably. JuliaArrays/OffsetArrays.jl#87. I don't have a solution yet but I haven't had time to poke at it (and neither has anyone else, apparently).

That doesn't look easy... but I guess until a larger change lands, this defn is OK?

mcabbott · 2020-03-30T10:13:02Z

could conceivably use TSlow for testing, since it has a custom similar method.

OK, that looks perfect. I see this replaces a T24Linear type, so perhaps I wait for #35298 to minimise churn.

Edit: I see it's merged already.

codecov-io · 2020-03-30T12:28:02Z

Codecov Report

Merging #35304 into master will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #35304      +/-   ##
==========================================
+ Coverage   86.81%   86.83%   +0.02%     
==========================================
  Files         344      344              
  Lines       63852    63832      -20     
==========================================
- Hits        55430    55426       -4     
+ Misses       8422     8406      -16

Impacted Files	Coverage Δ
base/bitset.jl	`91.74% <0%> (-0.98%)`	⬇️
stdlib/REPL/src/docview.jl	`84.97% <0%> (-0.87%)`	⬇️
base/range.jl	`87.88% <0%> (-0.24%)`	⬇️
stdlib/LinearAlgebra/src/blas.jl	`89.48% <0%> (-0.07%)`	⬇️
base/abstractarray.jl	`87.63% <0%> (-0.02%)`	⬇️
base/array.jl	`96.16% <0%> (-0.01%)`	⬇️
base/reflection.jl	`88.26% <0%> (ø)`	⬆️
stdlib/LinearAlgebra/src/LinearAlgebra.jl	`76.92% <0%> (ø)`	⬆️
stdlib/LinearAlgebra/src/cholesky.jl	`97.57% <0%> (ø)`	⬆️
stdlib/SparseArrays/src/sparsevector.jl	`95.5% <0%> (+0.09%)`	⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ddf79a8...1567f74. Read the comment docs.

test/arrayops.jl

timholy · 2020-04-06T08:25:37Z

Puzzling CI failure. Can you rebase on latest master?

mcabbott · 2020-04-06T11:34:57Z

Done, and mostly green ticks now.

timholy · 2020-04-06T11:45:49Z

Thanks for sticking with it, @mcabbott!

timholy reviewed Mar 30, 2020

View reviewed changes

mcabbott force-pushed the permutesimilar branch from 480a90e to 753eba6 Compare March 31, 2020 11:44

mcabbott commented Mar 31, 2020

View reviewed changes

test/arrayops.jl Outdated Show resolved Hide resolved

mcabbott mentioned this pull request Mar 31, 2020

Make gemm_strided_batched! work with PermutedDimsArrays JuliaGPU/CuArrays.jl#664

Closed

mcabbott pushed a commit to mcabbott/TransmuteDims.jl that referenced this pull request Mar 31, 2020

add similar like JuliaLang/julia#35304

d617058

mcabbott closed this Apr 5, 2020

mcabbott reopened this Apr 5, 2020

Michael Abbott added 4 commits April 6, 2020 10:27

Use parent for similar(::PermutedDimsArray)

a39d329

add test using TSlow array type

55085da

tidier isdefined check

66df48f

revert isdefined check

e6e7b69

mcabbott force-pushed the permutesimilar branch from 300df28 to e6e7b69 Compare April 6, 2020 08:27

timholy merged commit a965580 into JuliaLang:master Apr 6, 2020

mcabbott deleted the permutesimilar branch April 6, 2020 11:46

mcabbott mentioned this pull request Apr 6, 2020

similar(::PermutedDimsArray) JuliaLang/Compat.jl#693

Merged

ravibitsgoa pushed a commit to ravibitsgoa/julia that referenced this pull request Apr 9, 2020

Use parent for similar(::PermutedDimsArray) (JuliaLang#35304)

fb9aa38

ztultrebor pushed a commit to ztultrebor/julia that referenced this pull request Apr 17, 2020

Use parent for similar(::PermutedDimsArray) (JuliaLang#35304)

6aff213

staticfloat pushed a commit that referenced this pull request Apr 21, 2020

Use parent for similar(::PermutedDimsArray) (#35304)

beaa92e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use parent for similar(::PermutedDimsArray) #35304

Use parent for similar(::PermutedDimsArray) #35304

mcabbott commented Mar 30, 2020

timholy left a comment

timholy Mar 30, 2020

mcabbott Mar 30, 2020

mcabbott Mar 30, 2020

timholy Mar 31, 2020

mcabbott Mar 31, 2020

mcabbott commented Mar 30, 2020 •

edited

Loading

codecov-io commented Mar 30, 2020 •

edited

Loading

timholy commented Apr 6, 2020

mcabbott commented Apr 6, 2020

timholy commented Apr 6, 2020

		Base.axes(A::PermutedDimsArray{T,N,perm}) where {T,N,perm} = genperm(axes(parent(A)), perm)

		Base.similar(A::PermutedDimsArray, T::Type, dims::Base.Dims) = similar(parent(A), T, dims)

	similar(a::AbstractArray{T}) where {T} = similar(a, T)
	similar(a::AbstractArray, ::Type{T}) where {T} = similar(a, T, to_shape(axes(a)))
	similar(a::AbstractArray{T}, dims::Tuple) where {T} = similar(a, T, to_shape(dims))
	similar(a::AbstractArray{T}, dims::DimOrInd...) where {T} = similar(a, T, to_shape(dims))
	similar(a::AbstractArray, ::Type{T}, dims::DimOrInd...) where {T} = similar(a, T, to_shape(dims))
	# Similar supports specifying dims as either Integers or AbstractUnitRanges or any mixed combination
	# thereof. Ideally, we'd just convert Integers to OneTos and then call a canonical method with the axes,
	# but we don't want to require all AbstractArray subtypes to dispatch on Base.OneTo. So instead we
	# define this method to convert supported axes to Ints, with the expectation that an offset array
	# package will define a method with dims::Tuple{Union{Integer, UnitRange}, Vararg{Union{Integer, UnitRange}}}
	similar(a::AbstractArray, ::Type{T}, dims::Tuple{Union{Integer, OneTo}, Vararg{Union{Integer, OneTo}}}) where {T} = similar(a, T, to_shape(dims))
	# similar creates an Array by default
	similar(a::AbstractArray, ::Type{T}, dims::Dims{N}) where {T,N} = Array{T,N}(undef, dims)

	to_shape(::Tuple{}) = ()
	to_shape(dims::Dims) = dims
	to_shape(dims::DimsOrInds) = map(to_shape, dims)::DimsOrInds
	# each dimension
	to_shape(i::Int) = i
	to_shape(i::Integer) = Int(i)
	to_shape(r::OneTo) = Int(last(r))
	to_shape(r::AbstractUnitRange) = r

Use parent for similar(::PermutedDimsArray) #35304

Use parent for similar(::PermutedDimsArray) #35304

Conversation

mcabbott commented Mar 30, 2020

timholy left a comment

Choose a reason for hiding this comment

timholy Mar 30, 2020

Choose a reason for hiding this comment

mcabbott Mar 30, 2020

Choose a reason for hiding this comment

mcabbott Mar 30, 2020

Choose a reason for hiding this comment

timholy Mar 31, 2020

Choose a reason for hiding this comment

mcabbott Mar 31, 2020

Choose a reason for hiding this comment

mcabbott commented Mar 30, 2020 • edited Loading

codecov-io commented Mar 30, 2020 • edited Loading

Codecov Report

timholy commented Apr 6, 2020

mcabbott commented Apr 6, 2020

timholy commented Apr 6, 2020

mcabbott commented Mar 30, 2020 •

edited

Loading

codecov-io commented Mar 30, 2020 •

edited

Loading