-
-
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
Lazy broadcasts for reductions / more broadcasting customization docs #30939
Conversation
e9a797c
to
4bf723b
Compare
Yeah, I think this should just be a feature instead of a doc example. This is a really clever way of going about it - I had been thinking we’d use a macro and need to re-implement some of the lowering transform to get this done. Even though this doesn’t need a macro I think the macro interface makes a lot of sense still. |
Ok, then let the great bikeshedding begin. We could have
and export Regarding a macro: That might be nice, but I don't have any opinions on how this would look like. On syntax/naming, a loosely binding unary ascii operator would be perfect: The parentheses of We also need a warning in the docstring that There would be an argument for a variant of lazy broadcasts that eagerly checks In order to make reduction of lazy broadcasts perform well, we need to work a little on reductions. However, I would say that we can push this new feature, and later improve performance of typical reductions, as user reports come in. At least some |
I would actually like to see (Full disclosure, Peter and I have been working and thinking about lazy reductions and broadcast for a while now) |
Is there an issue/PR/archive/package with some results of your thoughts? Or should we use this PR for a discussion? Avoiding to lock in premature design decisions certainly makes sense. Then we would just put this as an example into the documentation, and potentially update it once the new feature is polished? |
This API is discussed in #19198. FYI, I also have a proof-of-concept implementation of a DSL for fused map-filter-reduce based on broadcast and Transducers.jl: https://github.com/tkf/Mathy.jl Note that using |
Because the question occasionally pops up on discourse, also gave a more explicit example for "wrapping in a Ref to behave like a scalar", and clarified that the convention |
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.
I still don't like documenting this before we implement it. If we're going to document it, we might as well just implement it in Base and export it.
Right now, only broadcast implementors are touching Broadcasted
objects. With this change we expose the API to a much larger surface and it becomes near-impossible to make meaningful changes. So either we decide that this is what we want and we export it, or we wait for more design iterations to be done.
The example is in the manual section for people who want to customize broadcasts. This necessitates a minimum understanding of the broadcasting machinery, and I find that copy-paste-able examples that can be used interactively are a great way to get started understanding something. Something like And while some minor changes of broadcast lowering would be great going forward (there were some issues where unneeded views where allocated), let's not kid ourselves that this could be done without breakage. Most of the other syntactic sugars have their lowering explained in the docs ( The explicit excerpt about wrapping in |
I think this is a good idea and it's better to focus on it rather than introducing utility functions/macros in the doc. I think it would be more appropriate to mention julia> Meta.@lower x .+ y .* z
:($(Expr(:thunk, CodeInfo(
@ none within `top-level scope'
1 ─ %1 = Base.broadcasted(*, y, z)
│ %2 = Base.broadcasted(+, x, %1)
│ %3 = Base.materialize(%2)
└── return %3
)))) and demonstrate how broadcasting expressions are evaluated. |
See my PR #31088 for adding the macro to |
Superseded by #31088. |
Another simple example for broadcast customization, that demonstrates how to reduce broadcast results without allocating intermediaries.
I am not sure; maybe the lazy broadcast even wants to become part of base, as a new feature (after bikeshedding with respect to names)?