Rename MLOperandDescriptor's "dimensions" key to "shape"

[a-sully]

Fixes #669

---

Preview | Diff

[a-sully]

@fdwr PTAL?

This is not a mass find-and-replace because, as I mentioned in #666 (comment):

• "dimensions" is ambiguous, since it can mean "some number of dimensions" (e.g. "the last two dimensions") or "shape" (e.g. [3, 4])

I've combed through all the uses of "dimensions" and updated uses of the latter definition to "shape" while keeping uses of the former definition as-is (though this process was manual and therefore fallible. Apologies if I missed anything!)

Thanks!

P.S. I'd like to follow up this change with #758 - I'll put up a PR shortly after this merges

[a-sully]

P.S. I'd like to follow up this change with #758 - I'll put up a PR shortly after this merges

P.P.S. I'd like to follow up that change with the changes proposed here #666 (comment)

[fdwr]

Sorry for the delay, and I realize this is tedious work. @huningxin and I talked about this some last week, and most importantly, we want to be sure it's done in a way that smoothly transitions existing samples, ORT Web, and people using the former to work on drivers, in a way that doesn't cause hiccups by allowing a grace period with the old name. It may still be an experimental API, but at least 24 people are affected these days by breaking changes.

My personal preference aside, we all agree with intra-API consistency is the most important, and so the current MLOperand::shape() and MLOperandDesc::dimensions disconnect should be resolved one way or another. Full considerations moved to #666 (comment).

[reillyeon]

I don't want to spend too much time bikeshedding on this but my reasons for preferring "shape" are:

1. "shape" is shorter and easier to spell.
2. "dimensions" could mean either "shape" or "rank" depending on whether it's interpreted as a vector or a scalar.

[a-sully]

we want to be sure it's done in a way that smoothly transitions existing samples, ORT Web, and people using the former to work on drivers, in a way that doesn't cause hiccups by allowing a grace period with the old name. It may still be an experimental API, but at least 24 people are affected these days by breaking changes.

Having a grace period where either dimensions or shape are accepted seems reasonable. We can discuss specifics on the Chromium CL, which I'll update shortly

I don't want to spend too much time bikeshedding on this but my reasons for preferring "shape" are:

Agreed, though I'll mention another developer-ergonomics reason I encountered while refactoring the WebNN WPTs, which is that since "dimensions" - while referring to one set of dimensions (i.e. one shape) - is plural, referring to a collection of dimensions is awkward and results in use of variable names like dimensionsArray, rather than just shapes

[zolkis]

(Thinking aloud). The way tensor definitions use these terms (for instance here) is that shape combines the use of rank (the number of indices required to get individual elements of a tensor), axis (the composing indices in the rank) and dimension(the number or elements on an axis).

Scalar: rank 0, no axes, no dimension, shape: ().
Euclidean vector (x, y, z): rank 1, one axis with 3 dimensions (on the 0th axis). Shape: (3).
2D matrix of 5 rows and 4 columns: rank 2, i.e. 2 axes (0 and 1), with given dimensions on rows (axis 0) and columns (axis 1). Shape: (5, 4).

In this sense, dimensions() would be ambiguous and shape() would be more exact (contrary to intuition in English).

[fdwr]

🤔 Have you considered sizes? It avoids the concern with dimensions (list of dimensions vs dimension count), it's equally short as shape and easy to spell, and it avoids the grammatical issues with shape ("... shape excludes information about the object's location, scale, orientation, and reflection... [unlike a] figure [which] is a representation including both shape and size..." [1]).

After collecting the pro's/con's above and elsewhere, and updating my comment here, I see why this was difficult, because there was no clear winner in this toss-up (if there was, there wouldn't be any discussion ⚖️).

Having a grace period where either dimensions or shape are accepted seems reasonable

Cool, that's really the most important aspect of this. I'm going to approve this because you're clearly intent your direction to avoid dimensions, and you're willing to do the breakage fixup work, but do also consider sizes.

[fdwr]

👀

[a-sully]

Thank you for the review! Please merge this at your convenience (the button is not available to me)

[fdwr]

Please merge this at your convenience

For stylistic and minor wording choices, I'm content to merge changes, but for bug fixes and content changes, I need my co-editor Ningxin's approval too.

[huningxin]

Thanks for putting this PR together. Just some minor clarification questions.

And could you also replace dimensions with shape in WebNN explainer: https://github.com/webmachinelearning/webnn/blob/main/explainer.md

[a-sully]

And could you also replace dimensions with shape in WebNN explainer: https://github.com/webmachinelearning/webnn/blob/main/explainer.md

Sure, I'll put up a follow-up PR shortly 👍

[huningxin]

LGTM!