Specify the operand data type constraints of operations by inexorabletash · Pull Request #646 · webmachinelearning/webnn

inexorabletash · 2024-04-18T14:16:35Z

Introduce constraints for input operands, either directly (e.g. input's dataType can only be "float32" or "float16") or indirectly (e.g. weight's dataType must be the same as input's).

Fixes #283

Preview | Diff

Introduce constraints for input operands, either directly (e.g. input's dataType can only be "float32" or "float16") or indirectly (e.g. weight's dataType must be the same as input's). Fixes #283

huningxin

LGTM with a nit, thanks much!

index.bs

huningxin

LGTM, thanks!

index.bs

huningxin

LGTM, thanks for the fix!

inexorabletash · 2024-04-25T00:34:55Z

Conflict resolved. @fdwr, good to merge?

inexorabletash · 2024-04-25T20:33:18Z

I'd missed reduceMean() somehow. Addressed in bd9b0cf

huningxin

LGTM

fdwr

Thanks JB. Tedious work there! I update GEMM C's data type check (please review). The rest LGTM, and my ideas can wait.

index.bs

fdwr · 2024-04-26T05:30:56Z

index.bs

    <div algorithm>
    The <dfn method for=MLGraphBuilder>reduceProduct(|input|, |options|)</dfn> method steps are:
-        1. Let |output| be the result of running the [=MLGraphBuilder/reduce-op | create reduce operation=] given "reduceProduct", |input| and |options|.
+        1. Let |output| be the result of running the [=MLGraphBuilder/reduce-op | create reduce operation=] given "reduceProduct", |input|, |options|, and « {{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}, {{MLOperandDataType/"uint32"}} ».


I like how you define them as type lists here, e.g. {{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}, {{MLOperandDataType/"int8"}}. Other specs I've seen that list supported data types (some don't and just leave it to you as the user to try it and find out! 🎲🤞) declare them as lists, rather than explicit prose (e.g. [=MLOperand/dataType=] is not {{MLOperandDataType/"float32"}} or {{MLOperandDataType/"float16"}}). I'm not suggesting you change those above right now, but if we start adding more type lists to more places, it will be nice to have them consistent.

Yeah, I raised this over in #643 (comment) - i.e. using the list style everywhere. I'm a fan and happy to change things to this form everywhere:

If input’s dataType is not in « "float32", "float16" », then ...

If input’s dataType is not in « "float32", "float16", "int32", "int8" », then ...

... with the question of which formulation to use if there's a single type:

If input’s dataType is not "uint8", then ...

If input’s dataType is not in « "uint8" », then ...

Thoughts? (Depends on later conversation, though.)

I guess a type list of 1 type is still a type list. So for consistency imo, If input’s dataType is not « "uint8" », then ....

index.bs

fdwr · 2024-04-26T05:56:37Z

index.bs

  </summary>
    1. If [=MLGraphBuilder/validating operand=] with [=this=] and any of |a| and |b| returns false, then [=exception/throw=] a {{TypeError}}.
+    1. If |a|'s [=MLOperand/dataType=] is not {{MLOperandDataType/"float32"}} or {{MLOperandDataType/"float16"}}, then [=exception/throw=] a {{TypeError}}.
+    1. If |b|'s [=MLOperand/dataType=] is not equal to |a|'s [=MLOperand/dataType=], then [=exception/throw=] a {{TypeError}}.


Hmm, yeah, increasingly the more I read this, more I think we should just add a new section of "data types at a glance", rather than burying them inside the algorithm steps. The steps can still say "if data type is not one of the blessed types", but it would be easier to see what's what. e.g.

Compute the matrix product of two input tensors.

partial interface MLGraphBuilder {
MLOperand matmul(MLOperand a, MLOperand b);
};

Arguments:

a: an MLOperand. The first input tensor which is at least 2D.‌

b: an MLOperand. The second input tensor which is at least 2D.

Returns: an MLOperand. The output tensor that contains the matrix product of two input tensors.

Types:

a: float16, float32

b: same as a

Computes the matrix product of two input tensors as follows...

Whatcha think? Later CR? This CR? Alternate idea instead?

Let me try a parallel PR to this one that tries this. I have a love/hate relationship with the Arguments/Returns preamble; since it's not really rigorous I think of it as non-normative. Listing types out-of-band will need some formal structure, and e.g. "same as a" needs to be precisely defined (i.e. the actual data type needs to match)

Okay, @fdwr I put up #657 as an experiment, updating only batchNormalization and adding a definition, and including two variations (list and table), which I think captures what you were going for. It's a clone of this PR with one additional commit if you want to look at the diff.

The use of "allowed data types" links in the steps means the reader/developer needs to understand that there's a nearby table giving the constraint details - I didn't want to add explicit dfns/links for each table entry, but maybe we should. I didn't find exact precedent but this is vaguely similar to CSS property definitions.

~~The bikeshed for linking the argument is unfortunately very verbose.~~ I figured out a shorter syntax!

I've gotten some feedback from @a-sully and @reillyeon about the approaches, and what this could enable (e.g. merge w/ Arguments, make it define shape as well, ...).

I might suggest that we merge this PR, either as-is or with the everything is a list change suggested above, and then iterate on table approach. Having this PR together has already identified some implementation gaps so start with this then iterate?

I might suggest that we merge this PR, either as-is

Yep yep. I'll do that now.

make it define shape as well

That would be interesting. Some other specs I've read were hand-wavy about how the output shape calculations occur, leaving a lot of room for ~~bugs~~ interpretation in implementations. So, seeing that clearly conveyed, without needing to refer to some separate Javascript codebase, would be nice (but having the WPT reference implementation is also very valuable).

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I620653f574303e6f5b18d3b540b647f07e0cf64c

SHA: b5677b6 Reason: push, by fdwr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I620653f574303e6f5b18d3b540b647f07e0cf64c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5492307 Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Austin Sullivan <asully@chromium.org> Cr-Commit-Position: refs/heads/main@{#1293407}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Ia55a214e7ad281ec3c8911e9116f388fac209d05

As specified in webmachinelearning/webnn#646. Besides, this CL also fixes the throwing TypeError and migrates the unit tests to WPT for prelu. Bug: 328567884, 327337526, 328026885 Change-Id: I2c6c0097b8d4fdc8c92bd66d21abd2cbda91e030 Cq-Include-Trybots: luci.chromium.try:win11-blink-rel

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Id377fcf423f72e2dd6d7851fa979fdfa8caeefdc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5517439 Commit-Queue: Shiyi Zou <shiyi.zou@intel.com> Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1297321}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: If0ea256805b5cfebdf15e0727fbc91acab918bbb

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Icea0e4a586aee33d7993d6e72393412b981d26ab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5495876 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Cr-Commit-Position: refs/heads/main@{#1297332}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Id377fcf423f72e2dd6d7851fa979fdfa8caeefdc Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5517439 Commit-Queue: Shiyi Zou <shiyi.zou@intel.com> Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1297321}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Icea0e4a586aee33d7993d6e72393412b981d26ab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5495876 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Cr-Commit-Position: refs/heads/main@{#1297332}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3

As specified in webmachinelearning/webnn#646. Besides, this CL also fixes the throwing TypeError and migrates the unit tests to WPT for prelu. Bug: 328567884, 327337526, 328026885 Change-Id: I2c6c0097b8d4fdc8c92bd66d21abd2cbda91e030 Cq-Include-Trybots: luci.chromium.try:win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5495874 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Cr-Commit-Position: refs/heads/main@{#1297344}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: Icea0e4a586aee33d7993d6e72393412b981d26ab Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5495876 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Cr-Commit-Position: refs/heads/main@{#1297332}

As specified in webmachinelearning/webnn#646. Besides, this CL also fixes the throwing TypeError and migrates the unit tests to WPT for prelu. Bug: 328567884, 327337526, 328026885 Change-Id: I2c6c0097b8d4fdc8c92bd66d21abd2cbda91e030 Cq-Include-Trybots: luci.chromium.try:win11-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5495874 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Cr-Commit-Position: refs/heads/main@{#1297344}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: If0ea256805b5cfebdf15e0727fbc91acab918bbb Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5500424 Reviewed-by: Austin Sullivan <asully@chromium.org> Auto-Submit: Shiyi Zou <shiyi.zou@intel.com> Commit-Queue: Shiyi Zou <shiyi.zou@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1297393}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3 Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3 Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5520313 Reviewed-by: Austin Sullivan <asully@chromium.org> Commit-Queue: Lisha Guo <lisha.guo@intel.com> Reviewed-by: ningxin hu <ningxin.hu@intel.com> Cr-Commit-Position: refs/heads/main@{#1298443}

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I5e600bdc791ecd4530408b65c15dd20611c211a3 Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel

This reverts commit ef29e11bfb1f1f4eae0935fb58f0e4129620837b. Reason for revert: LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/5728899092709376 Sample build with failed test: https://ci.chromium.org/b/8748452022241283393 Affected test(s): [ninja://services:services_unittests/WebNNGraphImplBackendTest.BuildAndComputeSingleOperatorGather](https://ci.chromium.org/ui/test/chromium/ninja:%2F%2Fservices:services_unittests%2FWebNNGraphImplBackendTest.BuildAndComputeSingleOperatorGather?q=VHash%3A3db3852ce13cc70a) If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F5728899092709376&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F5520313&type=BUG Original change's description: > webnn: Enforce input data type constraints for gather indices > > As specified in webmachinelearning/webnn#646 > > Bug: 328567884 > Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3 > Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5520313 > Reviewed-by: Austin Sullivan <asully@chromium.org> > Commit-Queue: Lisha Guo <lisha.guo@intel.com> > Reviewed-by: ningxin hu <ningxin.hu@intel.com> > Cr-Commit-Position: refs/heads/main@{#1298443} > Bug: 328567884 Change-Id: Iab874e915f5603861929cdf07ba4c09698b76503 No-Presubmit: true No-Tree-Checks: true No-Try: true

This reverts commit ef29e11. Reason for revert: LUCI Bisection has identified this change as the cause of a test failure. See the analysis: https://ci.chromium.org/ui/p/chromium/bisection/test-analysis/b/5728899092709376 Sample build with failed test: https://ci.chromium.org/b/8748452022241283393 Affected test(s): [ninja://services:services_unittests/WebNNGraphImplBackendTest.BuildAndComputeSingleOperatorGather](https://ci.chromium.org/ui/test/chromium/ninja:%2F%2Fservices:services_unittests%2FWebNNGraphImplBackendTest.BuildAndComputeSingleOperatorGather?q=VHash%3A3db3852ce13cc70a) If this is a false positive, please report it at http://b.corp.google.com/createIssue?component=1199205&description=Analysis%3A+https%3A%2F%2Fci.chromium.org%2Fui%2Fp%2Fchromium%2Fbisection%2Ftest-analysis%2Fb%2F5728899092709376&format=PLAIN&priority=P3&title=Wrongly+blamed+https%3A%2F%2Fchromium-review.googlesource.com%2Fc%2Fchromium%2Fsrc%2F%2B%2F5520313&type=BUG Original change's description: > webnn: Enforce input data type constraints for gather indices > > As specified in webmachinelearning/webnn#646 > > Bug: 328567884 > Change-Id: I33eba7e1def430b1cb94e3e7a4868e82c5bbd9a3 > Cq-Include-Trybots: luci.chromium.try:mac14-blink-rel,mac14.arm64-blink-rel > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5520313 > Reviewed-by: Austin Sullivan <asully@chromium.org> > Commit-Queue: Lisha Guo <lisha.guo@intel.com> > Reviewed-by: ningxin hu <ningxin.hu@intel.com> > Cr-Commit-Position: refs/heads/main@{#1298443} > Bug: 328567884 Change-Id: Iab874e915f5603861929cdf07ba4c09698b76503 No-Presubmit: true No-Tree-Checks: true No-Try: true Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5528819 Reviewed-by: Austin Sullivan <asully@chromium.org> Reviewed-by: Phillis Tang <phillis@chromium.org> Commit-Queue: Austin Sullivan <asully@chromium.org> Cr-Commit-Position: refs/heads/main@{#1298718}

Specify the operand data type constraints of operations

a6afe10

Introduce constraints for input operands, either directly (e.g. input's dataType can only be "float32" or "float16") or indirectly (e.g. weight's dataType must be the same as input's). Fixes #283

huningxin approved these changes Apr 19, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

huningxin requested a review from fdwr April 19, 2024 07:37

inexorabletash added 2 commits April 19, 2024 09:18

Merge remote-tracking branch 'origin/main' into data-type-constraints

b10bb85

gruCell: bundle hiddenState in with other type validations

1d91d80

a-sully mentioned this pull request Apr 19, 2024

Introduce MLNumber for specifying numeric inputs of any type #647

Merged

huningxin approved these changes Apr 24, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into data-type-constraints

38f7755

fdwr reviewed Apr 24, 2024

View reviewed changes

index.bs Outdated Show resolved Hide resolved

Identity should accept all types

16e424c

huningxin approved these changes Apr 24, 2024

View reviewed changes

Merge remote-tracking branch 'origin/main' into data-type-constraints

64f9bbc

inexorabletash added 2 commits April 25, 2024 09:14

Merge remote-tracking branch 'origin/main' into data-type-constraints

26ea2c6

Add reduceMean restriction

bd9b0cf

huningxin approved these changes Apr 26, 2024

View reviewed changes

Update gemm to check c data type too

ad239bc

fdwr approved these changes Apr 26, 2024

View reviewed changes

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 26, 2024

webnn: Enforce input data type constraints for conv2d

0a52960

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I620653f574303e6f5b18d3b540b647f07e0cf64c

chromium-wpt-export-bot mentioned this pull request Apr 25, 2024

webnn: Enforce input data type constraints for conv2d web-platform-tests/wpt#45922

Merged

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Apr 26, 2024

webnn: Enforce input data type constraints for conv2d

79baf4c

As specified in webmachinelearning/webnn#646 Bug: 328567884 Change-Id: I620653f574303e6f5b18d3b540b647f07e0cf64c

inexorabletash mentioned this pull request Apr 26, 2024

Make operand data type and rank validation table-driven #657

Merged

fdwr merged commit b5677b6 into webmachinelearning:main Apr 26, 2024

github-actions bot added a commit that referenced this pull request Apr 26, 2024

Specify the operand data type constraints of operations (#646)

da4aadc

SHA: b5677b6 Reason: push, by fdwr Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

chromium-wpt-export-bot mentioned this pull request Apr 28, 2024

webnn: Enforce input data type constraints for gemm and matmul web-platform-tests/wpt#45945

Merged

chromium-wpt-export-bot mentioned this pull request May 7, 2024

webnn: Enforce input data type constraints for gather indices web-platform-tests/wpt#46129

Merged

chromium-wpt-export-bot mentioned this pull request May 9, 2024

webnn: Enforce input data type constraints for some reduce operators web-platform-tests/wpt#46180

Merged

chromium-wpt-export-bot mentioned this pull request May 9, 2024

Revert "webnn: Enforce input data type constraints for gather indices" web-platform-tests/wpt#46183

Merged

chromium-wpt-export-bot mentioned this pull request May 11, 2024

Reland "webnn: Enforce input data type constraints for gather indices" web-platform-tests/wpt#46217

Merged

huningxin mentioned this pull request Jun 11, 2024

Bugfix: Add missing 64-bit integers support for some reduction operators #695

Merged

dontcallmedom mentioned this pull request Jan 15, 2026

CR Snapshot Update Request for Web Neural Network API - webnn w3c/transitions#769

Closed

Conversation

inexorabletash commented Apr 18, 2024 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

inexorabletash commented Apr 25, 2024

Uh oh!

inexorabletash commented Apr 25, 2024

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

fdwr left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fdwr Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

inexorabletash Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

fdwr Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

fdwr Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

inexorabletash Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inexorabletash Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

inexorabletash Apr 26, 2024

Choose a reason for hiding this comment

Uh oh!

fdwr Apr 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

inexorabletash commented Apr 18, 2024 •

edited by pr-preview bot

Loading

inexorabletash Apr 26, 2024 •

edited

Loading

inexorabletash Apr 26, 2024 •

edited

Loading

inexorabletash Apr 26, 2024 •

edited

Loading

fdwr Apr 26, 2024 •

edited

Loading