task: add broadcast class implementation - #2901
Conversation
jharlow-intel
changed the title
aa39048 to
1b251bf
Compare
1b251bf to
a2f50c3
Compare
|
View rendered docs @ https://intelpython.github.io/dpnp/pull/2901/index.html |
|
Array API standard conformance tests for dpnp=0.21.0dev5=py314ha0e2e8e_12 ran successfully. |
| if len(dpnp_arrays) > 1: | ||
| exec_q = dpt.get_execution_queue( | ||
| tuple(array.sycl_queue for array in dpnp_arrays) | ||
| ) | ||
| if exec_q is None: | ||
| raise dpt.ExecutionPlacementError( | ||
| "Execution placement can not be unambiguously inferred " | ||
| "from input arguments." | ||
| ) |
There was a problem hiding this comment.
do we need to check for compute follows data here? The arrays can be broadcast even if they aren't, it just means device routines can't be run with both as inputs
There was a problem hiding this comment.
if we do keep this check, we would need to check if they are dpnp.tensor.usm_ndarray as well
| The number of iterators. | ||
|
|
||
| """ | ||
| return len(self._arrays) |
There was a problem hiding this comment.
primarily a question for @antonwolfy and @vlad-perevezentsev since it's design-related, but NumPy and CuPy differ drastically in this class implementation
https://numpy.org/doc/2.1/reference/generated/numpy.broadcast.html
https://docs.cupy.dev/en/latest/reference/generated/cupy.broadcast.html
do we want more of the CuPy or NumPy behavior? What is the intended use-case of this class to users?
There was a problem hiding this comment.
I'd say we are not going to fully align with NumPy here, aligning with CuPy looks more preferable as for me, because might help in case of CuPy to DPNP migration for some users.
In that perspective, I'd keep the current implement as it is for now, plus adding values property, which mimics CuPy.
|
coverage: 78.475% (+0.02%) from 78.457% — task/SAT-7028 into master |
| The number of iterators. | ||
|
|
||
| """ | ||
| return len(self._arrays) |
There was a problem hiding this comment.
I'd say we are not going to fully align with NumPy here, aligning with CuPy looks more preferable as for me, because might help in case of CuPy to DPNP migration for some users.
In that perspective, I'd keep the current implement as it is for now, plus adding values property, which mimics CuPy.
Add an Attributes section to the dpnp.broadcast class docstring so the attributes (shape, size, ndim, numiter, values) render on the class page the way numpy/cupy present them, and drop the now-redundant per-attribute autosummary entries from the reference. Also document broadcast_arrays' variadic parameter as *args for consistency with broadcast/broadcast_shapes.
Use a dedicated autosummary template for the broadcast class that keeps the Attributes rubric visible (instead of the default template which hides member tables), so shape/size/ndim/numiter/values render under an "Attributes" heading like numpy/cupy, rather than the "Variables" label produced by a napoleon Attributes docstring section. Also add "numiter" to the docs spell-check word list.
Prefix the autosummary entries with ~ so the Attributes table lists the attributes by their short names (shape, size, ndim, numiter, values) instead of the fully qualified broadcast.<name> form.
antonwolfy
left a comment
antonwolfy
left a comment
There was a problem hiding this comment.
Hope it will be rendered properly now.
Thank you @jharlow-intel for the main implementation.
4 participants
Adds a
broadcastclass implementation