Specialize diag(::Diagonal)

[putianyi889]

The current implementation diag(::Diagonal, k=0), for the sake of type stability, returns a Vector even when D.diag is lazy.

julia> diag(Diagonal(1:5))
5-element Vector{Int64}:
 1
 2
 3
 4
 5

This can cause bugs when one does want the result to be lazy:

julia> D1 = Diagonal(1:5)
5×5 Diagonal{Int64, UnitRange{Int64}}:
 1  ⋅  ⋅  ⋅  ⋅
 ⋅  2  ⋅  ⋅  ⋅
 ⋅  ⋅  3  ⋅  ⋅
 ⋅  ⋅  ⋅  4  ⋅
 ⋅  ⋅  ⋅  ⋅  5

julia> D2 = Diagonal(1.0:5.0)
5×5 Diagonal{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}:
 1.0   ⋅    ⋅    ⋅    ⋅ 
  ⋅   2.0   ⋅    ⋅    ⋅
  ⋅    ⋅   3.0   ⋅    ⋅
  ⋅    ⋅    ⋅   4.0   ⋅
  ⋅    ⋅    ⋅    ⋅   5.0

julia> oftype(D2,D1)
ERROR: MethodError: Cannot `convert` an object of type Vector{Int64} to an object of type StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.83%. Comparing base (959d985) to head (9072335).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
- Coverage   91.84%   91.83%   -0.01%     
==========================================
  Files          34       34              
  Lines       15357    15358       +1     
==========================================
  Hits        14104    14104              
- Misses       1253     1254       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jishnub]

I'm unsure if diag(D) and diag(D,0) returning different types is a good idea. The specific conversion issue may be solved by simply copying the diagonal vector in the constructor:

Diagonal(A::AbstractMatrix) = Diagonal(_diagcopy(A))
Diagonal{T}(A::AbstractMatrix) where T = Diagonal{T}(_diagcopy(A))
Diagonal{T,V}(A::AbstractMatrix) where {T,V<:AbstractVector{T}} = Diagonal{T,V}(_diagcopy(A))

_diagcopy(D) = diag(D)
_diagcopy(D::Diagonal) = copy(D.diag)

with this,

julia> oftype(D2,D1)
5×5 Diagonal{Float64, StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}}:
 1.0   ⋅    ⋅    ⋅    ⋅ 
  ⋅   2.0   ⋅    ⋅    ⋅ 
  ⋅    ⋅   3.0   ⋅    ⋅ 
  ⋅    ⋅    ⋅   4.0   ⋅ 
  ⋅    ⋅    ⋅    ⋅   5.0

We may still want this, but let's be sure that it isn't for a quick fix.

[putianyi889]

Maybe zero-step ranges could be returned as substitutions of FillArrays.Fill. Then diag(::Diagonal{_,<:AbstractRange},k) becomes type-stable. For other lazy vectors, an interface similarzeros could be created.

[jishnub]

I think JuliaLang/julia#51475 may help with that

[putianyi889]

I think JuliaLang/julia#51475 may help with that

sort of. We need zero ranges with different lengths here.

[jishnub]

#1424 would offer an alternative to this.