Skip to content

improve docs of broadcasting section #47529

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

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

Conversation

bicycle1885
Copy link
Member

This improves the documentation of broadcasting, which proposes:

  • To stop using random numbers for reproducibility
  • To expand the explanation of how broadcasting makes conformal dimensions with more examples
  • To add a footnote about the difference from that of NumPy

@giordano giordano added docs This change adds or pertains to documentation broadcast Applying a function over a collection labels Nov 10, 2022
@johnnychen94
Copy link
Member

A similar/equivalent work #44170 as a reference but this one seems more readable. 😢


```
Broadcasting of a and B:
fill expand
Copy link
Contributor

@mcabbott mcabbott Nov 10, 2022

Choose a reason for hiding this comment

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

Here (and above) you say "fill" and "expand". But fill means something else. Perhaps clearer to use always the verbs which are commands: "reshape" and "repeat".

I also think the paragraph above could be much shorter. It need not read the whole table out in prose before showing the table. Just show the table, find one sentence to introduce it? Keep the table as close as possible to the examples above where we can see the results.

I'd also make c not re-use the numbers from B, so that the results are visually more distinct. Maybe it should even be c = [10 100 1000;;;] so that the example also introduces dimensions beyond the 2nd, even if they are trivial.

The fact that the first two dimensions (of 3 or more) are printed as being a matrix is what fits well with the start-at-the-beginning alignment of broadcast dims.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I think "fill" is less confusing because it is obvious from the context that it is used as a general English word here. Using "reshape" or "repeat" might be more confusing because functions with these names exist in Julia and the reader might misunderstand that reshape or repeat is called when broadcasting arrays.

The paragraph might look verbose but I think we should provide enough explanation to introduce a new concept. While the behavior of broadcasting is almost obvious to us, it is not always the case to others. I think this paragraph is important for readers to confirm their mental model in words.

I've changed c to [10 100 1000]. [10 100 1000;;;] looks distracting to me because the syntax is rarely used.

@bicycle1885
Copy link
Member Author

@mcabbott I've refreshed the pull request entirely, reflecting your suggestions. Can you check it again?

@bicycle1885
Copy link
Member Author

bicycle1885 commented Dec 8, 2022

Any chance to get this merged?

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Dec 8, 2022
@LilithHafner
Copy link
Member

@gbaraldi gbaraldi removed the triage This should be discussed on a triage call label Jan 5, 2023
@LilithHafner
Copy link
Member

Triage differs to @mcabbott

@vtjnash vtjnash added the merge me PR is reviewed. Merge when all tests are passing label Oct 30, 2023
@mcabbott
Copy link
Contributor

Triage differs to @mcabbott

Sorry I missed this!

  • I still think this could be shorter & clearer. Above I complain that there's a whole paragraph of prose telling you what the table is about to tell you.

  • What's the reasoning for "align then expand" as two steps? (repeat is one step, just above.)

  • This introduces rather a lot of new terminology. We are meant to infer what "align", "expand", "lacking dimensions", "singleton dimension", and "fills" (the second dimension of A) mean. Some of these are concepts which have existing names in Julia, repeat is used just before. Can't we avoid many of them?

  • If I didn't already know, I'm not sure I'd come away from this clearly understanding why [1,2] .+ [3,4,5,6] can't work, repeat would be fine. When doesn't broadcasting work, can this be made to seem obvious?

I wonder a bit if the whole idea of virtual repeats can just be scrapped. We need not think of [1,2,3] ./ 4 as repeating the 4, it's just accessed repeatedly: [x[i]/4 for i in 1:3]. Maybe we only need to describe the rule for working out the final size of the output, that's where you need to know which end to "align" sizes. After that how it's filled in is just like x./4 really. broadcast(+, A, B, C) is equal to [A[i] + B[i,j] + C[1,1,k] for i in 1:2, j in 1:3, k in 1:4], although I don't know whether that's been introduced yet.

@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
broadcast Applying a function over a collection docs This change adds or pertains to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants