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.

Sign up for GitHub

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

Jump to bottom

RFC: Use @: to construct a broadcasted object #31088

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

RFC: Use @: to construct a broadcasted object #31088

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1d2970b
Allow @: as a macro
tkf Feb 9, 2019
00f257d
Use @: to construct a non-materialized broadcasted object
tkf Feb 16, 2019
d06ce50
Merge remote-tracking branch 'origin/master' into at-colon
tkf Apr 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 26 additions & 1 deletion base/broadcast.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ using .Base.Cartesian
using .Base: Indices, OneTo, tail, to_shape, isoperator, promote_typejoin,
_msk_end, unsafe_bitgetindex, bitcache_chunks, bitcache_size, dumpbitcache, unalias
import .Base: copy, copyto!, axes
export broadcast, broadcast!, BroadcastStyle, broadcast_axes, broadcastable, dotview, @__dot__
export broadcast, broadcast!, BroadcastStyle, broadcast_axes, broadcastable, dotview, @__dot__, @:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe export+document a renamed _lazy as well? Hiding this functionality behind a macro only does not seem right. But the macro is really nice syntax!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reviewing the code!

I think it's better to avoid overly increasing the API surface. I don't see what exporting Lazy enables you to do other than what is already possible with @:. Also, implementing Lazy can be done in a few lines so I don't think Base should export it. (Note that it still makes sense for Base to export @:, even though it can be done in a few lines, because it let us use a common syntax across Julia ecosystem.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The main advantage of having a macro is nice looking source.

The cost of macros in general is increased complexity: It is not immediately clear what the AST is, what the types are, etc. Hence, readers of code need to either trust the docs (that don't tell you what they do on the AST), read the macro code, or macro-expand. In this specific case, the macro does not do anything complicated, and could be written as a lazy.(...). I am surely not the only one who tries to use macros sparingly, and I think lazy is important enough that it should be accessible to people who prefer view and dot-syntax or even explicit broadcast over @view, @. and the like.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree on the point that it is ideal to provide non-macro interface for language features whenever appropriate. It certainly makes sense for normal functions. However:

  1. Not every feature in Julia has "normal syntax" equivalent. For example, @inline, @inbounds, @simd, etc. do not have a straight-forward non-macro syntax.

  2. You can always run @macroexpand or Meta.@lower to see what is going on in a given macro (or a specialized syntax). I think macros in Julia are very transparent.

  3. _lazy does very unusual thing in the evaluation of the broadcasting expression (because that's its purpose). There has to be something that signals the readers that the evaluation rule is changed, even if they never used this feature. A macro is the best syntax for signaling this because that's what macros do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough.


## Computing the result's axes: deprecated name
const broadcast_axes = axes
Expand Down Expand Up @@ -1211,4 +1211,29 @@ end
end
@inline broadcasted(::S, f, args...) where S<:BroadcastStyle = Broadcasted{S}(f, args)

function _lazy end # only used in `@:` macro; does not have to be callable
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_lazy <: Function does not seem right.

struct _lazy end and an exported const Lazy = _lazy() with broadcasted(::_lazy, x) = (x,) could be an alternative (users would write either sum(@: a .* b) or sum(Base.Broadcast.instantiate(Lazy.( a.* b)))).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does hold the place of a function, though. Either which way, it's necessarily a bit of a strange construct, so I'm not sure it really matters.

I don't want to see it exported for exactly that reason. It's odd (but necessary).

# wrap the Broadcasted object in a tuple to avoid materializing
@inline broadcasted(::typeof(_lazy), x) = (x,)

"""
@: broadcasting_expression

Construct a non-materialized broadcasted object from a `broadcasting_expression`
and [`instantiate`](@ref) it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this as being akin to a generator; that is, generator : comprehension :: Broadcasted : broadcast. Perhaps that could give us a bit of a better language around this? Maybe call it a broadcast generator?

Copy link
Member Author

@tkf tkf Feb 21, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that "non-materialized broadcasted object" is a bit too mouthful. It's nice to have a concise word for it.

broadcast generator

Broadcasted is kind of like generators but I'm seeing the generators as a way to define iterators (i.e., limited forms of the coroutines). From this point of view, I find it somewhat confusing. But then Base.Generator is just a lazy version of map in Julia and it's pretty close to Broadcasted. So maybe it's fine? I slightly prefer "lazy broadcast" although the term "lazy" is too overloaded.

generator : comprehension :: Broadcasted : broadcast

By the way, what do : and :: mean here? Maybe Base.Generator corresponds to Broadcasted and [f(x) for x in xs] corresponds to broadcast(f, xs)?

Copy link
Member

@mbauman mbauman Feb 28, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those colons aren't code... it's a (now antiquated) way of writing a chained analogy. For example puppy : dog :: kitten : cat can be read "a puppy is to a dog as a kitten is to a cat." Or in other words, the relationship between a generator and a comprehension is comparable to the relationship between Broadcasted and broadcast. I shouldn't have written it like that — someone would only have reason to know that if they specifically took a particular US college entrance exam before 2005. 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's an interesting notation. Thanks for using it 😄


# Examples
```jldoctest
julia> bc = @: (1:3).^2;

julia> bc isa Broadcast.Broadcasted # it's not an `Array`
true

julia> sum(bc) # summation without allocating an array
14
```
"""
macro (:)(ex)
return esc(:($instantiate(($_lazy.($ex))[1])))
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super cool syntax!

I like the fact that instantiate is called before passing the object on. If a non-macro variant gets exported, I think that should eagerly instantiate as well. That is, @inline broadcasted(::typeof(_lazy), x) = (instantiate(x),) instead of instantiating in the macro? Or am I missing some reason that this cannot work?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, that's possible. Maybe better to do less in the macro. Another way is to create a custom wrapper object and instantiate in materialize, like you did in the other PR. But I'm seeing this as an implementation detail and it's better to use the simplest one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless _lazy gets exported in some form, I agree.


end # module
1 change: 1 addition & 0 deletions base/exports.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -973,6 +973,7 @@ export

@assert,
@__dot__,
@:,
@enum,
@label,
@goto,
Expand Down
7 changes: 4 additions & 3 deletions src/julia-parser.scm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2330,9 +2330,10 @@
(disallowed-space '@ nxt)))
(with-space-sensitive
(let ((startloc (line-number-node s))
(head (if (eq? (peek-token s) '|.|)
(begin (take-token s) '__dot__)
(parse-atom s #f))))
(head (case (peek-token s)
((|.|) (begin (take-token s) '__dot__))
((:) (take-token s))
(else (parse-atom s #f)))))
(peek-token s)
(if (ts:space? s)
(maybe-docstring
Expand Down
5 changes: 5 additions & 0 deletions test/syntax.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1832,3 +1832,8 @@ end
allocs = @allocated identity(p)
@test allocs == 0
end

@test Meta.parse("@: a b c") == Expr(:macrocall,
Symbol("@:"),
LineNumberNode(1, :none),
:a, :b, :c)