-
-
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
Document that reduce-like functions use init
one or more times
#53945
base: master
Are you sure you want to change the base?
Conversation
Co-authored-by: mikmoore <[email protected]>
Co-authored-by: Steven G. Johnson <[email protected]>
and add more clarifications
Do we want to specify whether it's |
I don't think there's a meaningful reason to do so, because subsequent steps might be Edit: Oh duh, that's not true. You sometimes have two results. Duh. But my point is that you never have two elements, and that which one may be a value from the collection is unspecified. But now I need to clean up that language some more. Shucks. 🤦 |
I think I had a non-associative example in mind, for which I should have used |
This docstring feels awfully long, especially when viewed in |
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.
Looks great, thank you for taking this on @mbauman. I just have one remaining point of disagreement about assuming purity of f
and then I'm fully on board.
I'm a bit late to the party, but after consideration and discussion with @mbauman, I'm on board with this change. I think the guarantee can be expressed more clearly this way:
I agree that the wording of the new docs is a bit verbose, but we can always worry about that after the functionality is nailed down. |
The argument ordering is very hard to express without explicitly invoking an
|
I don't like requiring that |
Is " |
As an example, yes. |
Try to make each paragraph here "punchier" and more pointed: * First paragraph more specifically describes the general behavior * Second paragraph is about `init`, clarify what it means for `op` args * Third paragraph is about `op`, but point to extended help * Fourth paragraph is about empty collections The extended help now reads clearer with a bulleted list and explicitly mentions argument ordering... and is also a bit more careful about floating point associativity (or the lack thereof) in mapreduce
init
one or more timesinit
one or more times
OK, I've pushed a new re-wording. I find this more clear, personally. We seem have a very solid consensus that "reduce-like functions use init one or more times" so I've removed the RFC from the title. I think I've faithfully represented that idea in the docs with this latest push. Of course comments on the wordings are still very welcome, but I am much happier with the state of this now. |
and try to fix ≈ docs
It makes me a tad anxious that we don't guarantee whether the |
No: https://en.wikipedia.org/wiki/Semigroup#Identity_and_zero |
So a monoid is required, not just associativity. |
I have two reasons why that's not a major concern in my view. One is that there are three "types" of arguments that get passed to
Because the The second is more simplistic, but it really seems like |
Perhaps more succinctly, just look at If you care about argument orderings between |
When the operation is |
Well the previous documentation explicitly says that it may or may not be used... but it'd probably be a good idea to get an idea of how many folks were relying on the old implementation. I say we first try the "full pairwise" in #52397 so we can run PkgEval there to gather the data. A deprecation path would be kind but would probably have to be fairly finely scoped (the generic |
One behavior I'd like to preserve is julia> sum(fill(-0.0, 0))
0.0
julia> sum(fill(-0.0, 3))
-0.0
I would be somewhat unhappy if a sum of ( |
I've become completely radicalized against initial value guesswork when it's not specified, and the proposed behavior change here is only about what to do when Guessing at
In other words, neither |
OK, here's take two, building on the work that was done in #53871. We preserve the neutral element requirement but guarantee that
init
is used to start all evaluation chains.The news summarizes as succinctly as I can:
And the docs for
mapreduce
say a bit more:Closes #49042.