Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename MLOperandDescriptor's dimensions to shape #669

Closed
a-sully opened this issue Apr 30, 2024 · 2 comments · Fixed by #676
Closed

Rename MLOperandDescriptor's dimensions to shape #669

a-sully opened this issue Apr 30, 2024 · 2 comments · Fixed by #676
Assignees

Comments

@a-sully
Copy link
Contributor

a-sully commented Apr 30, 2024

As discussed in #666:

MLOperandDescriptor takes a dimensions field, while querying this field from an MLOperand is done with the shape() method. As per https://w3ctag.github.io/design-principles/#attribute-reuse, we should re-use terms rather than unnecessarily introducing new vocabulary.

A quick CTRL-F of the spec text shows:

  • 139 instances of "dimensions", mostly due to MLOperandDescriptor.dimensions
  • 483 instances of "shape", which seems to be used more commonly in prose (e.g. "the shape of the input tensor")

We should decide to use one or the other (and possibly continue this discussion on another issue regardless)

"shape" is the more widely used term, so I propose we align on that. WDYT?

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 10, 2024
As proposed in webmachinelearning/webnn#669

This CL was created exclusively using targeted find-and-replaces,
followed by running git cl format. This CL has no behavioral changes,
though it is a breaking change for ~all consumers of WebNN

Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel,win11-blink-rel
Change-Id: I6b37ebc505a37686709f006b3ecafcefd3e2016d
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 10, 2024
As proposed in webmachinelearning/webnn#669

This CL was created exclusively using targeted find-and-replaces,
followed by running git cl format. This CL has no behavioral changes,
though it is a breaking change for ~all consumers of WebNN

Bug: 365813262
Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel,win11-blink-rel
Change-Id: I6b37ebc505a37686709f006b3ecafcefd3e2016d
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Sep 11, 2024
As proposed in webmachinelearning/webnn#669

This CL was created exclusively using targeted find-and-replaces,
followed by running git cl format. This CL has no behavioral changes,
though it is a breaking change for ~all consumers of WebNN

Bug: 365813262
Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel,win11-blink-rel
Change-Id: I6b37ebc505a37686709f006b3ecafcefd3e2016d
@fdwr
Copy link
Collaborator

fdwr commented Sep 17, 2024

shape is widely used by other ML frameworks

Seems like a tossup, as "dimensions" is widely used by ML libraries too: XNNPACK dims, ANN dimensions, DML DimensionCount and Sizes, MPS MPSNDArraySizes, CudNN CUDNN_ATTR_TENSOR_DIMENSIONS, OneDNN dnnl_dims_t. 🤷‍♂️

"dimensions" is ambiguous, since it can mean [rank or list of dimensions] ...

Interestingly that ambiguity also applies to a field named cats, as you can have a list of cats (std::vector<Cats> cats = {"Fido", "Garfield", "Heathcliffe"}) or a count of cats (uint32_t cats) depending on vector vs scalar usage. 😺 Really though, we should shore up the specification to consistently use rank when we mean rank, not sometimes loosely refer to "dimensions" to mean rank.

Updated my comment with everyone's pro's/con's statements so far, but I have one other idea on #676 (comment)... 🔍.

@zolkis
Copy link
Collaborator

zolkis commented Sep 18, 2024

🤔 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]).

Using sizes instead of dimensions indeed avoids some confusion, but adds some other... and it's less "standard" (de facto) than shape is. All these terms are overloaded. Since Web NN is targeting framework developers, and the term shape is well known among them (list of tensor component dimension counts, whose length is the rank and whose indices are the axes), I'd slightly prefer shape, but I'd not be confused using sizes, provided we include definitions in the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants