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

Add wrapper for passing constants through type inference #9452

Merged
merged 1 commit into from
Dec 27, 2014
Merged

Conversation

timholy
Copy link
Member

@timholy timholy commented Dec 24, 2014

It occurred to me that this doesn't have its own PR yet. This is so trivial, let's try to bikeshed the name fairly quickly (we've all had some months to think about this).

@rfourquet
Copy link
Member

+1 from one of the users of Val who wait for it to land into master.

@kmsquire
Copy link
Member

+1!

@andreasnoack
Copy link
Member

#9196 served as a trial balloon for using a special character for this () and I wouldn't call the response enthusiastic so I'd say that Val is our best candidate right now. I'm in favor of merging soon.

@kmsquire
Copy link
Member

+1

@kmsquire
Copy link
Member

Whoops... forgot I already +1'd this... Anyway, I like Val as well. Cheers!

timholy added a commit that referenced this pull request Dec 27, 2014
Add wrapper for passing constants through type inference
@timholy timholy merged commit cf8e27d into master Dec 27, 2014
@timholy
Copy link
Member Author

timholy commented Dec 27, 2014

OK, now set in stone :-).

@rfourquet
Copy link
Member

Val{:D}

@StefanKarpinski
Copy link
Member

Val{:D}

cute :-)

@timholy timholy deleted the teh/val branch December 29, 2014 16:10
@quinnj
Copy link
Member

quinnj commented Jan 1, 2015

Should we add any docs about this? I think I missed a larger discussion somewhere about what this does, so I feel a little in the dark. How can one use this?

@johnmyleswhite
Copy link
Member

For one example, you can use it to dispatch on Bool values.

@andreasnoack
Copy link
Member

I'm preparing a pull request where I use them to construct triangular matrices, e.g. Triangular(A, Val{:L}) to construct a lower triangular matrix.

@timholy
Copy link
Member Author

timholy commented Jan 1, 2015

See #9540.

@andreasnoack, note that in the documentation I created instances rather than types in function calls. We should probably pick one or the other and use it consistently. I chose this way largely because Jeff has expressed this as his (mild) preference.

@andreasnoack
Copy link
Member

In the cases where I'm considering Val, I think that the type version is better because you just want to specify a type parameter, e.g. :L for lower triangular. Furthermore, Val{:L} is already a bit long and Triangular(A, Val{:L}()) just doesn't feel delicious. I'm beginning to prefer LowerTriangular, but that is a different issue.

@timholy
Copy link
Member Author

timholy commented Jan 2, 2015

I confess I largely agree with you---using types decreases the burden at the call site.

This is something that perhaps others should chime in on; if we're not consistent, it will be frustrating for users.

@johnmyleswhite
Copy link
Member

I strongly prefer using types rather than instances for this entire pattern, above and beyond the specific case of Val. I'm already frustrated by the inconsistencies raised by this pattern throughout Julia-land: in Distances.jl and GLM.jl, you need to use instances; but in Distributions.jl, you can often use types. In many cases, the use of instances seems totally gratuitous since the properties of the default instance are completely ignored and the dummy instance, being immutable, can't be reused as a return value.

@kmsquire
Copy link
Member

kmsquire commented Jan 2, 2015

Is there a performance reason to choose one over the other?

@johnmyleswhite
Copy link
Member

I'm not really sure, but I don't believe so. I think @JeffBezanson has reasons for being unhappy with the "types as tags" patterns, perhaps related to the accumulation of an excessive number of different methods. But it seems to me like we're already doing things with the type system that weren't quite intended and that Val and the proposed trait system all depend on something that is similar to "types as tags" in spirit.

@rfourquet
Copy link
Member

What about defining Val(x) = Val{x}() to reduce verbosity at call site? (assuming the type instability is OK).

@andreasnoack
Copy link
Member

@rfourquet The reason for introducing Val is type stability, so that definition would defeat the purpose.

@tonyhffong
Copy link

A syntactic sugar related question: what do you all think about using this macro for symbol-valued Val{T}, i.e.

macro §_str(s)
    isa(s, AbstractString) ||
        error("@§_str only accepts a literal string")
    Expr( :curly, :Val, QuoteNode( symbol( s ) ) )
end

so Triangular( A, Val{:L} ) can be written as Triangular( A, §"L" ).

Pros:

  • we avoid confusing Val{:L} with Val(:L)
  • If commonly used, we preemptively discourage newer users from trying things like
a = :L
Triangular( A, Val{a} )

which defeats the whole purpose (at least a significant part of the purpose, I think).

  • Visually it hints at something unusual about type argument, kind of like how @ hints at something special about macros. Also it stands out a bit more in editors where strings are highlighted differently. (I'd suspect most editors color parentheses and curlies the same). Well, it stands out as a tag.

Cons:

  • This only affects usage. We still have to use Val{T} in declaration, i.e. f{T}( ::Val{T} ) = .... So the difference between declaration and usage feels asymmetric.

(note: § is not a valid label character in julia, but you can type it in REPL using \S[tab])

@andreasnoack
Copy link
Member

I tried with in #9196, but maybe § is different.

@tonyhffong
Copy link

@andreasnoack oops, thanks for pointing out.

@quinnj
Copy link
Member

quinnj commented Jan 2, 2015

I wonder if this all points to needing a nice, Julian implementation of enums that would serve these purposes. Enums could serve as the official, "dispatchable" values/instances. They could also be useful for implementing Tim Holy's traits as well along with the cases being discussed here.

@JeffBezanson
Copy link
Member

Just as Tim said, it's a mild preference. I won't object to Val{x} over Val{x}(). Two of the things I don't like about types-as-tags are (1) needing to declare subtypes to get different "values", and (2) with instances, you can easily add internal state to them in the future if necessary. Neither of those apply in this case.

@timholy
Copy link
Member Author

timholy commented Jan 3, 2015

OK, let's go with Val{x} then. Don't have time at the moment, but I'll fix the docs and re-commit them.

@timholy
Copy link
Member Author

timholy commented Jan 3, 2015

Thanks for chiming in, Jeff!

timholy added a commit that referenced this pull request Jan 4, 2015
This reverts commit ef7f5d4.

This is modified to incorporate the verdict in the discussion
of #9452.
@toivoh
Copy link
Contributor

toivoh commented Jan 14, 2015

Coming a bit late to the party, but type instability of Val(x) = Val{x}() shouldn't be a problem as long as the constructor call is inlined (which it really should be), right?

It's nicer to type ::Val{:X} than ::Type{Val{:X}} at the definition site. Also, if we want a consistent pattern for this sort of thing, I agree with Jeff that instances are the way to go. Otherwise we will have to debate where to draw the line between using types and instances over and over again.

@timholy
Copy link
Member Author

timholy commented Jan 15, 2015

@toivoh, that doesn't work:

julia> import Base.Val

julia> @inline Val(x) = Val{x}()
Val{T}

julia> arraytype{N}(::Type{Val{N}}) = Array{Float32,N}
arraytype (generic function with 1 method)

julia> arraytype{N}(::Val{N}) = Array{Float32,N}
arraytype (generic function with 2 methods)

julia> f() = arraytype(Val(3))
f (generic function with 1 method)

julia> g() = arraytype(Val{3})
g (generic function with 1 method)

julia> Base.return_types(f, ())
1-element Array{Any,1}:
 Type{_<:Array{Float32,N}}

julia> Base.return_types(g, ())
1-element Array{Any,1}:
 Type{Array{Float32,3}}

@toivoh
Copy link
Contributor

toivoh commented Jan 15, 2015

Aha. Thanks for checking!
Does anyone know why? Could it?

@rfourquet
Copy link
Member

Ah yes thanks for checking, I wanted to but it wasn't clear to me how to do it.

@timholy
Copy link
Member Author

timholy commented Jan 15, 2015

I presume this basically comes down to a difference between expression-interpolation and inlining. I suspect this fails at the first step, on type inference of Val(3), because 3 is a value rather than a type and Val{T<:Int} doesn't tell you anything useful. Without that, it can't look up "which method" (yes, there is only one...) of Val() it will be using. So it falls back to the slow generic path, and once 3 is known at run time it proceeds from there.

It does seem possible that this could be "fixed" (with the risk, of course, that it would break other stuff and not be acceptable). If you're interested in pursuing this, if I were you I'd look into the order of events in inference.jl. Presumably the inlining_pass comes after type inference has decided it understands what's going in; one might be able to add a special case, in cartoon form:

    if length(available_methods) == 1 && popmeta!(bodyexpr(available_methods[1])), :inline)[1]
        # there's only one method, and it's been declared @inline.
        # So just interpolate the expression in
        ...
    end

I don't know inference.jl well enough to be confident of this, unfortunately, and don't have time to look into it further.

But if you want this changed, it would be best if you or someone looked into this before Val gets so widely used it becomes painful to change.

@andyferris
Copy link
Member

Am I correct in saying that @pure Val(x) = Val{x}() now works quite well?

The consequence is that callers using Val(x) and callees using ::Val{x} seems like much simpler syntax to me. I've been using this form for traits in various packages, like Size(S)::Size{S} in StaticArrays.

It seems to me that @pure might make us revisit the general advice to use singleton types rather than instances. xref https://discourse.julialang.org/t/singleton-types-vs-instances-as-type-parameters/2802

@andyferris
Copy link
Member

Also we can define for instance show(io, ::Val{x}) where {x} = print(io, "Val($x)") for better visuals.

@andyferris
Copy link
Member

See #22475 for the suggested changes.

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

Successfully merging this pull request may close these issues.