Fix unidirectionally broadcast the shapes steps

[huningxin]

Fix #662

---

Preview | Diff

[fdwr]

The current definition is correct given the interpretation of the first/input tensor (A) being broadcasted to the second/target tensor (B), and the text elsewhere in the spec matches that interpretation. So for expand, the inputShape (A) is broadcasted to newShape (B), and for prelu, the slope (A) is broadcasted to the input (B) shape. Current text:

partial interface MLGraphBuilder {
  MLOperand expand(MLOperand input, sequence<[EnforceRange] unsigned long> newShape);
};
...
Set outputDescriptor.dimensions to the result of unidirectionally broadcasting the shapes input’s shape and newShape.
...
partial interface MLGraphBuilder {
  MLOperand prelu(MLOperand input, MLOperand slope);
};
...
Set descriptor.dimensions to the result of unidirectionally broadcasting the shapes slope’s shape and input’s shape.

So if we're going to flip the interpretation, then we'd need to adjust the wording in two places too:

- Set outputDescriptor.dimensions to the result of unidirectionally broadcasting the shapes input’s shape and newShape.
+ Set outputDescriptor.dimensions to the result of unidirectionally broadcasting the shapes newShape and input’s shape.
...
- Set descriptor.dimensions to the result of unidirectionally broadcasting the shapes slope’s shape and input’s shape.
+ Set descriptor.dimensions to the result of unidirectionally broadcasting the shapes input’s shape and slope’s shape.

I see for ONNX, rather than the parameter order of (input -> output) or (original shape -> target shape), it lists parameters as (target shape <- original shape), which feels oddly backwards to me, but I can accept that (technically both definitions are functionally equivalent, just a different parameter order). The bigger point of confusion is that A and B are simply unclear names, and we could pick some that are clearer. How about originalShape and targetShape or oldShape and newShape or another less ambiguous pair?

[huningxin]

@fdwr

So if we're going to flip the interpretation, then we'd need to adjust the wording in two places too:

We also need to change another caller "unidirectionally broadcastable":

-shapeA is unidirectionally broadcastable to shapeB if [unidirectionally broadcasting the shapes](https://www.w3.org/TR/webnn/#unidirectionally-broadcasting-the-shapes) shapeA and shapeB does not result in failure.
+shapeA is unidirectionally broadcastable to shapeB if [unidirectionally broadcasting the shapes](https://www.w3.org/TR/webnn/#unidirectionally-broadcasting-the-shapes) shapeB and shapeA does not result in failure.

And we need to flip one internal step of "unidirectionally broadcast the shapes":

-If dimA is not equal to dimB and dimA is not equal to 1, then return failure.
+If dimB is not equal to dimA and dimB is not equal to 1, then return failure.

[huningxin]

@fdwr

How about originalShape and targetShape or oldShape and newShape or another less ambiguous pair?

+1. I suppose this ambiguity comes from the readers cannot easily tell the broadcasting direction. How about using fromShape and toShape?

[huningxin]

The bigger point of confusion is that A and B are simply unclear names, and we could pick some that are clearer.

7f2c783 tries to use shapeFrom and shapeTo. Hope they are clearer. Please take another look. Thanks!

[inexorabletash]

I want to defer to @fdwr but call out:

If the claim "The current definition is correct given the interpretation of ..." is correct, and this change is turning into a strict (1) reorder and (2) rename of the params, then it seems like this line needs to be altered to maintain the current behavior.

Also, @fdwr's note about updating the "call sites" needs to happen as well?

[inexorabletash]

Can we move forward here? I'd recommend starting from a fresh change (force-push) that just renames the parameters from A and B (or is that B and A?) to shapeFrom and shapeTo and then we can decide if we want to swap the order or make other changes.