-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
WIP/RFC: Sweet Holy Traits #13222
WIP/RFC: Sweet Holy Traits #13222
Conversation
I've been thinking this a while... I wonder if it would be better to export a single macro as follows:
|
@hayd How difficult would that be compared to the 3 macros? It does seem a lot easier to remember/explain. |
Essentially you'd check the .head of the Expr passed in and decide which path to take. You'd probably want to check a couple of other conditions after that too (e.g. for :comparison .args[2] == :<: and .args[3] is a trait type), there are probably already some other sanity checks. |
The problem with your traitimpl syntax is that it doesn't generalize readily to multiparameter traits: @traitimpl Tr{Int,Float64} Also we may want to have trait inheritance at some point, say: @traitdef Tr2{X} <: Tr1{X}, Tr0{X,X} again clashing. That aside, I'm not sure that it would be less confusing, because those are different things. However, maybe drop the |
@@ -3995,17 +3995,6 @@ Multiply elements of an array over the given dimensions. | |||
prod(A, dims) | |||
|
|||
doc""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That shouldn't have been removed (yet), sorry.
I'm still of the opinion that there is a solution with a single macro, even if it's not quite as easy as above (or below). Could you do (granted, this has the extra repetition):
Is inheritance be implied i.e.
you could also define (again, if I've not misunderstood the syntax)
If I've misunderstood, what about (though I find doing two at once with just a comma/tuple a little confusing):
|
I think that using the same macro for multiple purposes is confusing. Compare e.g. to abstract types, and let's assume we'd want to implement them via macros. Would we really go for a syntax where @abstract AbstractString
@abstract type ASCIIString{; AbstractString} chars::Vector{Char} end
@abstract print{; AbstractString{T}}(str::T) = ... I think not. It's confusing, since one needs to carefully look at what is being defined to see whether it is a type, a function, or something else (i.e. an abstract type). In the absence of new syntax for traits (e.g. abstract types have the In the future, if traits become part of the Julia syntax, (some of) these traits macros would be replaced by operators or a suitable inline syntax. |
I disagree that the following is better.
The goal is making traits 1st class so if it's possible to agree on valid syntax already (hopefully which won't without the need for parser changes) we should try to. |
The Thus I read |
I don't think this is ready for base Julia. That one needs to have a macro in front of a function definition to use this feature suggests that it's not fully reconciled with the rest of the language yet. It doesn't seem like an improvement on the existing method and I personally don't find it more readable (when I look at the HT implementation I can understand how it works just by understanding basic Julia, while this new macro implementation is full of mystery). |
Thanks for the comments @iamed2. A few thoughts: Concerning the macro-use: generated functions also use a macro in their definition, so this seems acceptable for Base, at least at times. In fact a lot of the functionality provided by Base is through macros, for instance all of parallel programming. Having said this, I suspect it would be reasonably easy to remove the necessity for Concerning the statement "It doesn't seem like an improvement on the existing method...". Macros needn't be a mystery thanks to
Also HTs can be implemented with some slight variations, so if they would become more common either they wouldn't be consistent or they would need to be checked manually for consistency. Anyways, I can see the following reasons to not include something like this PR in Base (there may well be more):
By gauging my JuliaCon conversations and github issues: 1 is at earliest in julia-0.6 and I reckon that neither 2 nor 3 are true. I cannot comment on 4. This is not intended to be the end of the trait-story but a stop-gap solution until a more complete solution is implemented. The advantage of including this is that traits could be used & tested in practical usage now. This would hopefully help guiding their final implementation. Note that this PR intentionally does not address the harder issues of trait definitions in terms of methods defined, of trait inheritance nor of trait dispatch. (edit: added point 1) |
22c16ca
to
f674018
Compare
This is very nice. Thanks for both the packages and this PR. As for whether it should go in base vs stay in a package, obviously a macro-based solution is not ideal when compared to a more integrated solution. So if it's going to go in now, one potential criterion for evaluation might be: if this went in, how many lines of code could you delete from Base? For example, I think the main reason Cartesian went into Base (aside from my misunderstanding about approval from Jeff and Stefan 😉) was because it resulted in a net removal of several hundred LOC. Do you have the sense that something similar could happen here? I think it's fair to say that you shouldn't have to count against your tally any lines of test code that are specifically testing the new trait functionality. But even if you omit those, right now the balance of +/- is not in your favor. |
Since Tim gave such thoughtful input, here's my two cents: (1) I really, really want traits to exist in Base one day. (2) I think you are going to continue to hit opposition to the use of macros. But I'm skeptical that making the parser changes required to make traits part of the official syntax will pay off, as I believe the complete design of traits will end up getting revised and not just the syntactic aspects. (3) I think there are lots of places in Base where traits could help clarify things. The most obvious one is the lack of an Iterator trait, which could substantially clarify the intent of a lot of code in Base (and outside of it). More generally, I think the way to achieve Tim's goal of something like code reduction is to demonstrate which traits we could add to Base to improve it. The one complication with an Iterator trait is that it's not necessarily a code reduction, as much as a safety improvement. But I think we currently have lots of code (in e.g. Distributions.jl) that would be more appropriately generic if you could talk about things like My suspicion with this PR is that it will play out a lot like your PR for a doc system. You'll end up spearheading the development of an essential addition to the language. You'll also help to clarify where things are easy and where things need more thought. And ultimately the system we end up with will look a lot like your demo. But the PR itself won't get merged -- it'll get folded into a larger PR as the rest of the community jumps onboard. FWIW, I think you should get an award for doing this kind of work. Even if this PR doesn't get merged, I think your influence on the language's development is unmistakeable. |
@johnmyleswhite What do we do in the meantime? I'm very concerned that, unless we get traits done right first in Base, that a lot of things like the array revamping will just want to be done yet again after traits get in. |
No one could say it better, @johnmyleswhite. @ScottPJones, it would be great to base the next round of array overhaul on traits. But we've already started that process with |
It is not clear to me that that a "real" traits system is just syntactic sugar if you want to support proper semantics for traits wrt to composition, flattening, composition order, and conflict resolution. Also, it seems a bit strange to make Trait's dynamic and not declarative, in that I could reload packages in a slightly different order and now dispatch is different. Also, since Traits will piggy back off our subtyping system you get certain properties for free, but not everything you may want (how do you express exclusive disjunction?). Lots of good work has been done in this area. I would suggest that such a large language feature be thought out very carefully instead of evolving in an ad-hoc way. Thanks to @mauro3 for kicking off the discussion as I do agree that some Trait like mechanism will eventually exist. |
Just want to chime in and thank @mauro3 for his work so far, even if this doesn't get merged. I continue to gain value from the Traits package on an almost-daily basis. |
@jakebolewski Do you have some suggestions on how traits should be done, to make it declarative instead of dynamic (I have a dread of dynamic things that could be lexical, since the days of MacLisp vs. Scheme!) |
Whoa, all these nice comments! Thanks a lot! @timholy, yes, pretty bad on the LOC front: 205 up vs at most 40 down if @johnmyleswhite, I had no idea anyone remembered my doc-ventures! I've been meaning to look into how traits could be used in Base but found no time so far. But I agree, that would help this along for sure. The trait for iteration, an important one, would be a bit more tricky to implement as that would require trait defs in terms of defined methods, i.e. in the Traits.jl territory. @jakebolewski: I think HTs could be used (and probably will be used) as a way to try out traits. Of course, that would mean that those bits would need to be overhauled as traits get implemented properly. But you're right, whereas this PR is just sugar, proper traits will have extra features. I'll need to ponder the other bits you alluded to. @ScottPJones for non-Base use, I think SimpleTraits.jl is usable now. In fact I should register it. @malmaud, thanks, let me know if you got more feedback for Traits.jl. |
This is indeed really awesome and needed. Thanks for pushing the envelope here, @mauro3! There's another pseudo-trait in the base library right now… but it's not a trait since it predates the THHT: typealias StridedArray{T,N,A<:DenseArray,I<:Tuple{Vararg{RangeIndex}}} Union{DenseArray{T,N}, SubArray{T,N,A,I}} This guy sorely needs to be replaced with a trait, but it's not as simple — it depends upon the parameters of SubArray. In general, I think this will be a common use of traits. The ability to make a wrapper type act more like the type its wrapping is really important. It would be interesting to explore more inheritance-like syntaxes, since I think that's how this might look as a first-class part of Julia. The trouble is that once you start making it look more familiar, you suddenly want composition and ambiguities to "just work" with more than one trait. @trait getindex(x::(AbstractArray+LinearFast), I...) = ...
@trait getindex(x::(AbstractArray+LinearFast+Strided), I...) = … And now we hit a wall. We need a type-theorist to properly define these intersections of Until that happens, I think we should keep spelling out traits the good old-fashioned way. Macros can help some, but if you actually take them the whole way there, they fall into this uncanny valley where you expect things to happen that simply can't be emulated. |
You (ab??)use Union (rather than +):
|
Surface syntax isn't the issue — it could be |
This is the opposite of a union type – it's an intersection type. The way to write it would be something like |
Yes, this is why Fortress had intersection types. On Wed, Sep 23, 2015 at 7:19 PM, Stefan Karpinski notifications@github.com
|
This PR cannot do intersections, however, Traits.jl can in two related ways. A trait can inherit from several others: @traitdef Tr{X} <: Tr1{X}, Tr2{X} begin
...
end So any @traitfn getindex{X<:AbstractArray; LinearFast{X}}(x::X, ...) =...
@traitfn getindex{X<:AbstractArray; LinearFast{X}, Strided{X}}(x::X, ...) =... (Note that in the second function essentially an ad-hoc trait is created which is the intersection of LinearFast and Strided. Or framed differently the trait inheriting from those two and not adding any other constraints.) But @mbauman is right, it becomes tricky to figure out dispatch, i.e. the most specific trait intersection a type belongs to. And in general ambiguities cannot be avoided. Traits.jl can do this kind of dispatch (probably not quite right though) see issue 5 where @tonyhffong and I discussed this and see the function containing the logic. Please experiment with it, it would be great to put together more test cases and document them better, the start is here. I think Traits.jl can do this in all generality thanks to staged functions, so it should be suited to prototype this. And indeed, a type-theorist's input is sorely needed. |
I doubt we need a true intersection type, because tuples do that for you:
Then we just need the parser to be smart enough to generate these methods from "convenient" declarations like foo{T,N}(X::AbstractArray{T,N}&LinearSlow) = maximum(X) We may need #265 first, though, because ordering of the traits in the tuple might be changed by future declarations. (Not sure this is really true, haven't thought about it seriously.) |
(In case it's not obvious, what I meant is that the ambiguities may be avoided by explicitly generating the proper nodes in the tree, much in the way that the existing parser suggests defining certain function signatures to avoid ambiguities.) |
Even using tuples of traits for dispatch, I don't quite see how the right traits can be selected in the wrapper function (i.e. do the dispatch). My example doesn't actually need trait-tuples but two not connected traits traitdefT1(::A) = Tr1() # the method doing the grouping for trait Tr1
traitdefT2(::B) = Tr2() # the method doing the grouping for trait Tr2
f(a::Any & Tr1) = 1
f(a::Any & Tr2) = 2 gets translated into two wrapper functions, the 2nd overwriting the 1st, so it can never dispatch to Tr1:
One could define a grouping function for both Tr1 and Tr2 but that does not scale as a new function would need to be defined for all used trait combinations. Traits.jl solves this using a generated function which accesses a store of all traits used in trait-methods of a method. But it could well be that I'm missing something? |
I would expand it as a single trait function with two trait-types. To be explicit: julia> abstract Trait1
julia> immutable Trait1A <: Trait1 end
julia> immutable Trait1B <: Trait1 end
julia> type A end
julia> type B end
julia> f(a) = _f(a, trait1(a))
f (generic function with 1 method)
julia> trait1(::A) = Trait1A()
trait1 (generic function with 1 method)
julia> trait1(::B) = Trait1B()
trait1 (generic function with 2 methods)
julia> _f(a, ::Trait1A) = 1
_f (generic function with 1 method)
julia> _f(a, ::Trait1B) = 2
_f (generic function with 2 methods)
julia> f(A())
1
julia> f(B())
2 |
I think an Intersect type would be clearer/more explicit, as well as less ambiguous with actual tuples. |
This pattern can be used to have a trait which groups into more than two groups by adding f(a::Any&Trait1A) = 1
f(a::Any&Trait1B) = 2 now I want to add a method
so I would need to code up a special grouping function:
Whilst it works, it does not scale. |
I wouldn't do it that way. I'd say it's a future version of julia's job to expand f(a) = _f((a, trait1(a), trait2(a), ...)) That's why I say we might need to solve #265 first; as soon as you add a new trait as an independent "dimension," you have to go back and recompile all the old functions that called (and perhaps inlined) |
The trait functions could be expressed in terms of calling a supertype with an abstract hierarchy defined by convention: abstract Trait
abstract LinearIndexing <: Trait
immutable LinearFast <: LinearIndexing; end
immutable LinearSlow <: LinearIndexing; end
call{A<:AbstractArray}(::Type{LinearIndexing}, ::Type{A}) = LinearSlow() # fallback default Then it's rather simple to identify traits ( Indeed, the rub is #265 and incrementally adding non-overlapping traits. |
Here's an interesting idea: what if we did away with independent locuses of traits and forced all traits for a given type to be defined at once? Then there would be just one trait accessor function, and it would return a e.g.,
Now, when we define a function that uses traits: @trait f(x::Intersection{AbstractArray, LinearSlow, Strided}) = body
# can lower to:
f(x::AbstractArray, args...) = _f_(x, traits(x), args...) # AbstractArray is the only T !<: Trait
_f_(x::AbstractArray, T::Tuple{Vararg{Union{LinearSlow, Strided}}}, args..) = $body
|
Yes, it does seem to require that the traits that are mutually-exclusive (like f(x::Intersection{AbstractArray, LinearSlow}, args...) = ... but later write f(x::Intersection{AbstractMatrix, PositiveDefinite, LinearFast}, args...) = ... which would need to be internally-rewritten for consistency with the first method. |
If you require all such traits to be defined together, then this may as well be a single statement (like an enum): abstract Trait = LinearSlow | LinearFast | Strided |
@mbauman, I don't think your last proposal would work with multi-parameter traits. I think those would be nice to have, although at the moment only single-parameter traits are used/envisioned for use. However, if only single-parameter traits are wanted, maybe an implementation along the lines of @Rory-Finnegan's https://github.com/Rory-Finnegan/Interfaces.jl could be used. To contrast with above suggestions, the approach I take in Traits.jl is the following (described here). When defining a trait function, say
I think this is more flexible than any of the ways outlined above: any number of traits can be used, no predefined list is needed. But I don't think that this is ready for Base, thus this is not in this PR. Anyway, something this discussion shows is that the design space for HTs is large. Thus introducing macro sugar to do them could ensure that we don't end up with a multitude of slightly different HTs. That would be much harder to transition to the proper trait-system-to-be than something consistent. |
So, I'll just mention that Interfaces.jl doesn't work with more recent versions of julia v0.4 due some changes to the union type in base julia. However, if folks just wanted the single-parameter traits in base it could be included in ~150 lines of julia code and ~50 lines of C code (for the mutable union). It would also work for defining interfaces for collections and data structures out of the box. |
I've improved SimpleTraits.jl (in this branch) incorporating dispatch on several traits as Tim suggested in this comment (this leads to about double the LOCs). So now this works for two traits @traitfn f55{X, Y; TT1{X}, TT2{Y}}(x::X, y::Y) # a traitfn now needs initialisation
@traitfn f55{X, Y; TT1{X}, TT2{Y}}(x::X, y::Y) = 1
@traitfn f55{X, Y; !TT1{X}, TT2{Y}}(x::X, y::Y) = 2
@traitfn f55{X, Y; TT1{X}, !TT2{Y}}(x::X, y::Y) = 3
@traitfn f55{X, Y; !TT1{X}, !TT2{Y}}(x::X, y::Y) = 4 As Tim said above, the slots cannot be mixed around. Note that above divides the set of all types into four sets (similarly, 3 traits give 8 groups, etc.). I think this would mean that traits dividing into more than two groups, as e.g. suggested by Matt here, wouldn't be necessary. Last, note that this addition is still not as flexible as the scheme I outlined above (as used by Traits.jl) as that scheme allows any combination of traits. |
I think a good way forward for this PR is:
|
The design space is indeed very large here. I've been trying to pare down the features to the bare minimum that would get us the most useful chunks of functionality. I still need to sit down with Traits.jl and learn how it works more thoroughly. Could you give an example where a multi-parameter trait would be used? That said, I'm not sure we should limit ourselves to binary on-off traits. As an example, I've been playing with RaggedArrays, and it's very nice to define LinearIndexing traits there that specifically iterate over ragged dimensions. The current system is nicely extensible for other types, too. |
Other possible traits in
|
@mbauman, a "classic" multiparameter trait would be equality and comparison:
and @eschnett: thanks for the many examples. I might have a look at the isequal-hash one. There was a big of discussion about how easy that is to break recently. Maybe that is something HT could help with. |
This PR implements macro-sugar for Holy Traits (HT). This is SimpleTraits.jl ported to Base (so to check it out, cloning SimpleTraits will be faster than building this PR).
The idea behind this PR is to make HT easier to use. An added benefit is that this marks them, so they could be easily updated once a more proper trait system is in place. (I suspect that the semantics of a trait-system-to-be will be a superset of HT, so the migration should be reasonably easy.) At the moment only the linear-indexing HT is in Base, but judging by the chat on julia-issues this may change (e.g. JuliaLang/LinearAlgebra.jl#255, JuliaLang/LinearAlgebra.jl#186, ...), thus a more structured approach may make sense.
The readme of SimpleTraits.jl gives an overview of the functionality. The short is:
which is quite a bit shorter and more readable than the manual implementation (as done for
LinearIndexing
in Base):For testing and illustrating, I've implemented some HT usages in Base (which should be purged again). The next step would be to refactor the
LinearIndexing
HT (the only HT currently used in Base, as far as I know). However, before embarking on that reasonably big task, I have this question: Does this PR have any chance of being merged? (Any other comments are of course welcome too!)Other things to consider:
TODO
LinearIndexing
traitRemove: