FillArrays.jl icon indicating copy to clipboard operation
FillArrays.jl copied to clipboard

`one(::Eye)` should return `Eye`

Open putianyi889 opened this issue 5 years ago • 2 comments

julia> one(Eye(∞))
ERROR: ArgumentError: Cannot fill! with 1.0 an AbstractFill with value 0.0.
Stacktrace:
 [1] fill! at C:\Users\DELL\.julia\packages\FillArrays\tE9Xq\src\FillArrays.jl:52 [inlined]
 [2] fill! at C:\Users\DELL\.julia\packages\LazyArrays\DfNL4\src\cache.jl:309 [inlined]
 [3] one(::Diagonal{Float64,Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.5\LinearAlgebra\src\special.jl:305
 [4] top-level scope at none:1

julia> copy(Eye(∞))
ERROR: OutOfMemoryError()
Stacktrace:
 [1] Array at .\boot.jl:406 [inlined]
 [2] similar at .\array.jl:379 [inlined]
 [3] _vec_resizedata!(::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::Int64) at C:\Users\DELL\.julia\packages\LazyArrays\DfNL4\src\cache.jl:128
 [4] resizedata! at C:\Users\DELL\.julia\packages\LazyArrays\DfNL4\src\cache.jl:139 [inlined]
 [5] resizedata! at C:\Users\DELL\.julia\packages\LazyArrays\DfNL4\src\cache.jl:114 [inlined]
 [6] setindex! at C:\Users\DELL\.julia\packages\LazyArrays\DfNL4\src\cache.jl:72 [inlined]
 [7] copyto_unaliased!(::IndexCartesian, ::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::IndexCartesian, ::Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}) at .\abstractarray.jl:874
 [8] copyto!(::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}) at .\abstractarray.jl:840
 [9] _copyto!(::LazyArrays.PaddedLayout{ArrayLayouts.DenseColumnMajor}, ::ArrayLayouts.OnesLayout, ::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}) at C:\Users\DELL\.julia\packages\ArrayLayouts\M5SFk\src\ArrayLayouts.jl:163
 [10] _copyto!(::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}) at C:\Users\DELL\.julia\packages\ArrayLayouts\M5SFk\src\ArrayLayouts.jl:167
 [11] copyto!(::LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}, ::Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}) at C:\Users\DELL\.julia\packages\ArrayLayouts\M5SFk\src\ArrayLayouts.jl:170
 [12] copyto!(::Diagonal{Float64,LazyArrays.CachedArray{Float64,1,Array{Float64,1},Zeros{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}}, ::Diagonal{Float64,Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}) at D:\buildbot\worker\package_win64\build\usr\share\julia\stdlib\v1.5\LinearAlgebra\src\diagonal.jl:73  
 [13] copymutable at .\abstractarray.jl:971 [inlined]
 [14] copy(::Diagonal{Float64,Ones{Float64,1,Tuple{InfiniteArrays.OneToInf{Int64}}}}) at .\abstractarray.jl:915
 [15] top-level scope at none:1

These statements are called when one wants Eye(∞)^0 and Eye(∞)^1.

putianyi889 avatar Nov 19 '20 22:11 putianyi889

I think for now this should be fixed in InfiniteArrays.jl via

one(D::Diagonal{T,<:AbstractFill{T,Tuple{OneToInf{Int}}}) where T = Eye{T}(size(D,1))
copy(D::Diagonal{T,<:AbstractFill{T,Tuple{OneToInf{Int}}}) where T = D

We could potentially add these here

one(D::Diagonal{T,<:AbstractFill}) where T = Eye{T}(size(D,1))
copy(D::Diagonal{T,<:AbstractFill}) where T = D

but that differs a bit from Base's behaviour for ranges so may cause other issues.

I'll move this issue to InfiniteArrays.jl.

dlfivefifty avatar Nov 19 '20 22:11 dlfivefifty

I've changed my mind: reading the documentation for one we should definitely have those overrides here:

If possible, one(x) returns a value of the same type as x

copy is a bit more questionable though.

dlfivefifty avatar Nov 20 '20 13:11 dlfivefifty

Both of the examples in the topmost post work now:

julia> using InfiniteArrays, FillArrays

julia> one(Eye(∞))
ℵ₀×ℵ₀ LinearAlgebra.Diagonal{Float64, Ones{Float64, 1, Tuple{InfiniteArrays.OneToInf{Int64}}}} with indices OneToInf()×OneToInf()

julia> one(Eye(∞)) == Eye(∞)
true

julia> copy(Eye(∞))
ℵ₀×ℵ₀ LinearAlgebra.Diagonal{Float64, Ones{Float64, 1, Tuple{InfiniteArrays.OneToInf{Int64}}}} with indices OneToInf()×OneToInf()

julia> copy(Eye(∞)) == Eye(∞)
true

julia> VERSION
v"1.9.3"

This works on v"1.6.7" as well, so perhaps there's nothing more to be done here.

jishnub avatar Aug 31 '23 05:08 jishnub