Skip to content

Clarify guidance on operations having data-dependent output shapes #193

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
Merged
merged 14 commits into from
Jun 28, 2021
Merged

Clarify guidance on operations having data-dependent output shapes #193

merged 14 commits into from
Jun 28, 2021

Conversation

kgryte
Copy link
Contributor

@kgryte kgryte commented Jun 7, 2021 •
edited
Loading

This PR supersedes gh-168 by @steff456.

In summary, this PR

  • clarifies guidance on functions/operations having data-dependent output shapes.
  • grants permission to array libraries which build array computation graphs to omit such operations.

Notes

  • I messed up the Git history of gh-168 when attempting to make updates. 😓

@kgryte kgryte mentioned this pull request Jun 7, 2021
Closed
@kgryte @shoyer
Update copy
f28fd4a 
Co-authored-by: Stephan Hoyer <[email protected]>
@jakirkham
Copy link
Member

Other operations can also wind up with unknown shapes like nonzero, unique, etc.. Not sure if we want to include these

@kgryte
Copy link
Contributor Author

kgryte commented Jun 7, 2021

@jakirkham This PR includes guidance for nonzero and unique.

@jakirkham
Copy link
Member

Sorry I must have missed that

@kgryte
Copy link
Contributor Author

kgryte commented Jun 7, 2021

@jakirkham No worries! 😅

leofang
leofang reviewed Jun 7, 2021
View reviewed changes
Copy link
Contributor

@leofang leofang left a comment

Choose a reason for hiding this comment

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

I have a question on the terminology. Is it common to say "array computation graphs"? I would have expected a more verbose phrasing:

  • libraries that build array computation graphs -> array libraries based on (or "that build") computation graphs

but maybe it's just a nitpick...

kgryte and others added 2 commits June 7, 2021 12:56
@kgryte
Copy link
Contributor Author

kgryte commented Jun 7, 2021

@leofang Updated copy based on your suggestions.

@rgommers rgommers added the Narrative Content Narrative documentation content. label Jun 28, 2021
rgommers
rgommers approved these changes Jun 28, 2021
View reviewed changes
Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

The text LGTM now, and the warnings are nice to have - let's get this in. Thanks for the input everyone!

@rgommers rgommers merged commit b7b0dcd into main Jun 28, 2021
@rgommers rgommers deleted the add-boolean-indexing-notes branch June 28, 2021 20:42
@rgommers rgommers added the topic: Dynamic Shapes Data-dependent shapes. label Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shoyer shoyer shoyer left review comments

@leofang leofang leofang left review comments

@rgommers rgommers rgommers approved these changes

Assignees
No one assigned
Labels
Narrative Content Narrative documentation content. topic: Dynamic Shapes Data-dependent shapes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@kgryte @jakirkham @rgommers @shoyer @leofang @steff456