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

Element type of `Diagonal{T}` is `T` even when `typeof(zero(T)) != T`

Open tkoolen opened this issue 7 years ago • 8 comments

From https://discourse.julialang.org/t/help-creating-a-jump-variable-array/11515/8?u=tkoolen. There are cases where typeof(zero(T)) != T, so what should the eltype of Diagonal{T} for such T?

One option would be to define

Base.eltype(::Type{Diagonal{T}}) where {T} = promote_type(T, typeof(zero(T)))

but this could lead to bugs where the T in Diagonal{T} is assumed to be the element type. Another option is to make it so that the Diagonal constructor converts the input vector to e.g. Vector{promote_type(T, typeof(zero(T)))} when necessary.

Maybe erroring in the constructor when typeof(zero(T)) != T could be another option.

tkoolen avatar Jun 08 '18 15:06 tkoolen

Having a type where zero(T) is not of type T seems highly pathological.

StefanKarpinski avatar Jun 08 '18 16:06 StefanKarpinski

but this could lead to bugs where the T in Diagonal{T} is assumed to be the element type.

Even worse is that Diagonal reports that T to AbstractArray{T} — leading to a strange inconsistency between eltype and functions that grab T from the AbstractArray directly. I think we have to assert that zero(T) is T.

mbauman avatar Jun 08 '18 16:06 mbauman

Strongly related: https://github.com/JuliaLang/julia/issues/27015

mbauman avatar Jun 08 '18 16:06 mbauman

leading to a strange inconsistency between eltype

Well, that's just wrong – but fixable :). If it needs a ComputedFieldType(:tm:), that is doable these days now. You just need the right incantation!. Here's an example: https://github.com/JuliaLang/julia/blob/917ae8bdad742e97751ed113d4464b95aae39c57/base/iterators.jl#L169

vtjnash avatar Jun 08 '18 16:06 vtjnash

Having a type where zero(T) is not of type T seems highly pathological.

I think what JuMP is doing is very reasonable (see Discourse link in description). JuMP.Variable just can't represent zero.

Also related: https://github.com/JuliaLang/LinearAlgebra.jl/issues/497, and actually also https://github.com/FugroRoames/Rotations.jl/issues/55.

I think @vtjnash's approach would work well on master, now that the 'storage' type parameter is separated from the 'element' type parameter (unlike how it was in 0.6); something like

function Diagonal(diag::V) where {T, V<:AbstractVector{T}}
    S = promote_type(T, typeof(zero(T)))
    new{S, V}(diag)
end

tkoolen avatar Jun 08 '18 17:06 tkoolen

Sometilmes I feel that eltype could be more of a trait for AbstractArray (or iterate, I suppose?). It makes sense as a type parameter of Array but I've noticed that carrying it around is inconvenient in some of the concrete arrays I've created.

On the other hand. removing T from AbstractArray and forcing everyone to use trait dispatch patterns just to capture the eltype might be too inconvenient at this stage?

andyferris avatar Aug 12 '18 19:08 andyferris

Is it still true that there are Base types for which typeof(zero(T)) != T?

It looks like some formerly inconsistent matrix zeros (e.g. https://github.com/JuliaLang/LinearAlgebra.jl/issues/170) are much more consistent nowadays

brenhinkeller avatar Nov 19 '22 19:11 brenhinkeller

Is it still true that there are Base types for which typeof(zero(T)) != T?

Yes, e.g. for T = Irrational{:π}. (Although Diagonal{Irrational{:π}} may not be of much practical importance, of course.)

martinholters avatar Nov 21 '22 13:11 martinholters