Skip to content

Conversation

@jishnub
Copy link
Member

@jishnub jishnub commented Sep 7, 2021

This PR adds a promote_rule definition for Diagonal matrices using the promotion rule for the parent diagonal vectors. I've also added a method Diagonal{T, V}(::Diagonal) to carry out necessary conversions. This differs from the constructor Diagonal{T,V}(::AbstractVector), and I hope that the usage is clear from the context. Currently the conversion throws an error, so this is not a breaking change.

This has two positives:

bugfix

Master

julia> D = Diagonal(1:3)
3×3 Diagonal{Int64, UnitRange{Int64}}:
 1    
   2  
     3

julia> D^0
ERROR: MethodError: no method matching Vector{Int64}(::Diagonal{Int64, UnitRange{Int64}})
Closest candidates are:
  Array{T, N}(::AbstractArray{S, N}) where {T, N, S} at array.jl:540
  Vector{T}() where T at boot.jl:467
  Array{T, N}(::StaticArrays.SizedArray{S, T, N, M, TData} where {M, TData<:AbstractArray{T, M}}) where {T, S, N} at /home/jb6888/.julia/packages/StaticArrays/vxjOO/src/SizedArray.jl:98
  ...
Stacktrace:
 [1] convert(#unused#::Type{Vector{Int64}}, a::Diagonal{Int64, UnitRange{Int64}})
   @ Base ./array.jl:532
 [2] Diagonal{Int64, Vector{Int64}}(diag::Diagonal{Int64, UnitRange{Int64}})
   @ LinearAlgebra /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/diagonal.jl:10
 [3] convert(T::Type{Diagonal{Int64, Vector{Int64}}}, m::Diagonal{Int64, UnitRange{Int64}})
   @ LinearAlgebra /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/special.jl:53
 [4] to_power_type(x::Diagonal{Int64, UnitRange{Int64}})
   @ Base ./intfuncs.jl:241
 [5] power_by_squaring(x_::Diagonal{Int64, UnitRange{Int64}}, p::Int64)
   @ Base ./intfuncs.jl:256
 [6] ^(A::Diagonal{Int64, UnitRange{Int64}}, p::Int64)
   @ LinearAlgebra /buildworker/worker/package_linux64/build/usr/share/julia/stdlib/v1.6/LinearAlgebra/src/dense.jl:447
 [7] macro expansion
   @ ./none:0 [inlined]
 [8] literal_pow(f::typeof(^), x::Diagonal{Int64, UnitRange{Int64}}, #unused#::Val{0})
   @ Base ./none:0
 [9] top-level scope
   @ REPL[298]:1

This PR

julia> D^0
3×3 Diagonal{Int64, Vector{Int64}}:
 1    
   1  
     1

type-stability

Currently on master

julia> [Diagonal([1]), Diagonal([1.0])]
2-element Vector{Diagonal}:
 [1;;]
 [1.0;;]

julia> [Diagonal([ones(Int,1,1)]), Diagonal([ones(1,1)])]
2-element Vector{Diagonal}:
 [[1;;];;]
 [[1.0;;];;]

The element types are not concrete, as promotion falls back to typejoin here.

After this PR

ulia> [Diagonal([1]), Diagonal([1.0])]
2-element Vector{Diagonal{Float64, Vector{Float64}}}:
 [1.0;;]
 [1.0;;]

julia> [Diagonal([ones(Int,1,1)]), Diagonal([ones(1,1)])]
2-element Vector{Diagonal{Matrix{Float64}, Vector{Matrix{Float64}}}}:
 [[1.0;;];;]
 [[1.0;;];;]

@jishnub jishnub marked this pull request as draft September 7, 2021 12:45
@jishnub jishnub marked this pull request as ready for review September 7, 2021 14:48
@jishnub
Copy link
Member Author

jishnub commented Sep 12, 2021

@dkarrasch would like your opinion on this

@dkarrasch dkarrasch added the linear algebra Linear algebra label Sep 12, 2021
@dkarrasch
Copy link
Member

@timholy Would you be able to take another quick look? @jishnub I haven't touched the tests, so this should still do what you wanted it to do?

@dkarrasch dkarrasch merged commit 3ef1f61 into JuliaLang:master Nov 12, 2021
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Feb 22, 2022
Co-authored-by: Daniel Karrasch <daniel.karrasch@posteo.de>
LilithHafner pushed a commit to LilithHafner/julia that referenced this pull request Mar 8, 2022
Co-authored-by: Daniel Karrasch <daniel.karrasch@posteo.de>
@jishnub jishnub deleted the promotediagonal branch August 17, 2022 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linear algebra Linear algebra

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants