- Notifications
You must be signed in to change notification settings - Fork54
feat: addbroadcast_shapes to the specification#983
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
rgommers left a comment
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.
Thanks@kgryte! Overall LGTM and seems good to add this function to the standard. A few comments with the "unknown shape" one the key thing to discuss.
| Returns | ||
| ------- | ||
| out: Tuple[int, ...] | ||
| a broadcasted shape. |
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.
This should probably say something like "The shape should match the regular broadcasting rules as documented in :ref:broadcasting" to make the specification a little bit more tight.
| - If not provided one or more arguments, the function **must** return an empty tuple. | ||
| .. note:: | ||
| Array libraries which build computation graphs (e.g., ndonnx and Dask) commonly support shapes having dimensions of unknown size. If a shape contains a value other than an integer (e.g., ``None`` for a dimension of unknown size), behavior is unspecified and thus implementation-defined. Array-conforming libraries **may** choose to propagate such values (e.g., if a shape contains a dimension size of ``None``, the returned broadcasted shape also has a corresponding dimension having a size equal to ``None``) or raise an exception. |
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.
This can be phrased such that it works for computation graphs in the expected manner I believe, since the broadcasting math is abstract anyway and any sentinels can propagate correctly. To be discussed in the next call I think.
lucascolley commentedDec 11, 2025
the array-api-extra docs just say |
This PR
broadcast_shapesto the specification #893 by adding support forbroadcast_shapesto the specification.Nonefor a dimension of unknown size, behavior is left unspecified and thus implementation-defined.