parametrize Diagonal on the wrapped vector type

[fredrikekre]

Because why not? Ref e.g. JuliaArrays/StaticArrays.jl#235

IIUC this could potentially be bad for people that now have ::Diagonal{T} fields in user defined types though, since theses types will not be fully parametrized?

If this is accepted, the same thing should probably be done with Bidiagonal and [Sym]Tridiagonal.

[fredrikekre]

Another idea, define something like

abstract type AbstractDiagonal{T} <: AbstractMatrix{T} end
struct Diagonal{T} <: AbstractDiagonal{T}
    diag::Vector{T}
end

and relax methods in base to AbstractDiagonal. Then users can subtype AbstractDiagonal like we currently subtype AbstractArray for our own types. The downside is that all users then need to use the same fieldname for the diagonal, since there are lots of direct field access to the vector. Or we need a standard way of obtaining the diagonal.

[fredrikekre]

Actually, we have diag for that, so all D.diag calls can be diag(D) instead.

[tkelman]

no blank between docstring and what it's documenting

[fredrikekre]

Woops, fixed.

[fredrikekre]

The thing with my alternative approach is that it is not enough to subtype AbstractVector but you have to introduce your own diagonal type and subtype that manually. The beauty with julia is usually that things JustWork™ with your own types, so I think this change is worth it.

Also, I searched through all registered packages, and this diff would fix the breakage part of this PR:

diff --git a/src/LowRankMatrix.jl b/src/LowRankMatrix.jl
index 7989736..a59da0e 100644
--- a/src/LowRankMatrix.jl
+++ b/src/LowRankMatrix.jl
@@ -8,7 +8,7 @@ Store the singular value decomposition of a matrix:
 """
 immutable LowRankMatrix{T} <: AbstractLowRankMatrix{T}
     U::Matrix{T}
-    Σ::Diagonal{T}
+    Σ::Diagonal{T,Vector{T}}
     V::Matrix{T}
     temp::Vector{T}
 end

Edit: And for Bidiagonal and [Sym]Tridiagonal, there is no usage of that in [mutable] structs in any registered package.

[tkelman]

not sure whether these are covered in benchmarks but let's see @nanosoldier runbenchmarks(ALL, vs = ":master")

[nanosoldier]

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

[c42f]

These should be elementwise equal - is this better expressed as all(Diagonal(linspace(1,3,3)) .== Diagonal([1.0,2.0,3.0]))?

I must admit to being surprised that 1:2 == [1,2] is false in the first place, as other AbstractVectors are compared elementwise. I hunted this down to #5778 which seems to be a bit of a thorny issue.

[fredrikekre]

Will presumably be fixed by #16401? @nalimilan

[nalimilan]

Right. As you say, that's a thorny issue... ;-) Until then, better use all indeed.

[fredrikekre]

Updated.

[Sacha0]

superficially lgtm! :)

[fredrikekre]

Rebased.

[fredrikekre]

Rebased.

[KristofferC]

Is this first use of triangular dispatch in base? Probably not...