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

Parametrizing a type for both a container-type field and its element type consistently #19476

Closed
wsshin opened this issue Dec 1, 2016 · 10 comments
Labels
domain:types and dispatch Types, subtyping and method dispatch

Comments

@wsshin
Copy link
Contributor

wsshin commented Dec 1, 2016

One of the performance tips entitled "Avoid fields with abstract containers" introduces a nice way to parametrize a type for both a container-type field and its element type. In its conclusion, the tip suggests using type definitions like:

type MyBetterContainer{T<:Real, A<:AbstractVector}
    a::A

    MyBetterContainer(v::AbstractVector{T}) = new(v)
end
MyBetterContainer(v::AbstractVector) = MyBetterContainer{eltype(v),typeof(v)}(v)

Such definition was suggested as a way to close a "hole" through which users can access the inner constructor with an element type T inconsistent with the actual element type of a container type A. For example, an attempt to instantiate MyBetterContainerwith a container type Vector{Float64} and element type Int64 (rather than Float64) generates an error:

julia> MyBetterContainer{Int64,Vector{Float64}}(1.:5)
ERROR: MethodError: Cannot `convert` an object of type FloatRange{Float64} to an object of type MyBetterContainer{Int64,Array{Float64,1}}
This may have arisen from a call to the constructor MyBetterContainer{Int64,Array{Float64,1}}(...),
since type constructors fall back to convert methods.
 in MyBetterContainer{Int64,Array{Float64,1}}(::FloatRange{Float64}) at ./sysimg.jl:53

However, the hole is not completely closed, because we can still do

julia> MyBetterContainer{Int64,Vector{Float64}}(1:5)
MyBetterContainer{Int64,Array{Float64,1}}([1.0,2.0,3.0,4.0,5.0])

Here, 1:5 instead of 1.:5 is passed. What happens is

  • because 1:5 is a subtype of AbstractVector{Int64}, it passes the type checking (v::AbstractVector{T} with T = Int64) at the boundary of the inner constructor, and
  • once 1:5 passes the boundary and reaches the constructor body, it is successfully assigned to a::A with A = Vector{Float64} because automatic type conversion occurs in field assignment.

Here are a couple of questions:

  1. How can we close this hole?
  2. Why doesn't Julia allow a type definition like the following?
type MyBetterContainer{T<:Real, A<:AbstractVector}
    a::A{T}
end

I can imagine that supporting a type definition like this is more complicated, but to me it seems like an ultimate and clean way to prevent inconsistency between T and the element type of A.

@kshyatt kshyatt added the domain:types and dispatch Types, subtyping and method dispatch label Dec 1, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Dec 1, 2016

@yuyichao yuyichao closed this as completed Dec 1, 2016
@yuyichao
Copy link
Contributor

yuyichao commented Dec 1, 2016

And #18457 should also make it possible to constraint A with T.

@wsshin
Copy link
Contributor Author

wsshin commented Dec 1, 2016

@yuyichao Could you elaborate on why I don't need T at all for my case? The Performance Tips documentation that I linked in the beginning shows the cases where I need T, i.e., when I want to "declare different versions of the outer function for different element types of a".

@yuyichao
Copy link
Contributor

yuyichao commented Dec 1, 2016

type MyBetterContainer{A<:AbstractVector}
    a::A
    MyBetterContainer(v::AbstractVector) = new(v)
end
MyBetterContainer{T<:AbstractVector}(v::T) = MyBetterContainer{T}(v)

@wsshin
Copy link
Contributor Author

wsshin commented Dec 1, 2016

@yuyichao Sorry, probably I edited my previous comment too late and you missed my reference to the Performance Tips documentation. Your example is basically equivalent to

type MyBetterContainer{A<:AbstractVector}
    a::A
end

and "Avoid fields with abstract containers" in the Performance Tips documentation exactly starts with it, and gradually evolve it into more sophisticated ones. In doing so, the documentaion provides convincing reason why in some cases we want to introduce T.

@wsshin
Copy link
Contributor Author

wsshin commented Dec 1, 2016

And the documentation claims that the last example shown there (MyBetterContainer in the documentation) closes the hole explained in my original posting, but I am saying that there is still a hole. I'm curious how to close the hole.

If there is no way to close the hole, I think the documentation should mention it, because it gives a false impression that MyBetterContainer doesn't have the hole.

@yuyichao
Copy link
Contributor

yuyichao commented Dec 1, 2016

You still don't need that type parameter if it's just providing redundant information. You can just replace all use of T by eltype(A). There are of course cases that requires extra parameters in the type (e.g. SubArray). Those parameters should not be specified by the user so there's no "hole".

@wsshin
Copy link
Contributor Author

wsshin commented Dec 1, 2016

I don't think all usage of T can be replaced by eltype(A). For example, what if you want to dispatch different functions depending on T, like the following example (which is shown in the previously mentioned documentation)?

function myfunc{T<:Integer, A<:AbstractArray}(c::MyContainer{T,A})
    return c.a[1]+1
end

function myfunc{T<:AbstractFloat, A<:AbstractArray}(c::MyContainer{T,A})
    return c.a[1]+2
end

@yuyichao
Copy link
Contributor

yuyichao commented Dec 1, 2016

myfunc{A}(c::MyContainer{A}) = myfunc_impl(c, eltype(A))

@wsshin
Copy link
Contributor Author

wsshin commented Dec 1, 2016

Thanks for the explanation! So it seems that the documentation went a bit too far in terms of playing with parameters, even though it was an interesting read...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:types and dispatch Types, subtyping and method dispatch
Projects
None yet
Development

No branches or pull requests

3 participants