Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
tkoolen opened this issue Jun 8, 2018 · 8 comments
Open

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

tkoolen opened this issue Jun 8, 2018 · 8 comments

Comments

@tkoolen
Copy link
Contributor

tkoolen commented Jun 8, 2018

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.

@StefanKarpinski
Copy link
Member

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

@mbauman
Copy link
Member

mbauman commented Jun 8, 2018

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
Copy link
Member

mbauman commented Jun 8, 2018

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

@vtjnash
Copy link
Member

vtjnash commented Jun 8, 2018

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:

Pairs(data::A, itr::I) where {A, I} = new{eltype(I), eltype(A), I, A}(data, itr)

@tkoolen
Copy link
Contributor Author

tkoolen commented Jun 8, 2018

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: JuliaLang/LinearAlgebra.jl#497, and actually also JuliaGeometry/Rotations.jl#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 tkoolen changed the title Element type of Diagonal Element type of Diagonal{T} is T even when typeof(zero(T)) != T Jun 8, 2018
@andyferris
Copy link
Member

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?

@brenhinkeller
Copy link
Contributor

brenhinkeller commented Nov 19, 2022

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. JuliaLang/LinearAlgebra.jl#170) are much more consistent nowadays

@martinholters
Copy link
Member

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants