Add the builder.input() method steps by zolkis · Pull Request #364 · webmachinelearning/webnn

zolkis · 2023-03-22T13:37:39Z

Add the input() method algorithm.
Depends on #337

zolkis · 2023-04-20T13:32:59Z

huningxin · 2023-05-10T05:18:25Z

index.bs

+        1. If |name| is `undefined` or an empty [=string=], then throw a "{{TypeError}}" {{DOMException}} and stop.
+    1. Let |descriptor| be the second argument.
+        1. If |descriptor| is not an an object that [=implements=] {{MLOperandDescriptor}}, then throw a "{{TypeError}}" {{DOMException}} and stop.
+        1.  If the [=byte length=] of |descriptor| is not supported by the underlying platform, then throw a "{{DataError}}" {{DOMException}} and stop.


nit: one extra space before "If"

huningxin · 2023-05-10T05:20:37Z

index.bs

+    1. Let |descriptor| be the second argument.
+        1. If |descriptor| is not an an object that [=implements=] {{MLOperandDescriptor}}, then throw a "{{TypeError}}" {{DOMException}} and stop.
+        1.  If the [=byte length=] of |descriptor| is not supported by the underlying platform, then throw a "{{DataError}}" {{DOMException}} and stop.
+        1. If the [=check dimensions=] steps given |descriptor|.{{MLOperandDescriptor/type}} and |descriptor|.{{MLOperandDescriptor/dimensions}} return `false`, throw a "{{DataError}}" {{DOMException}} and stop.


What would happen if user code passes a descriptor for scalar operand (dimensions is undefined)? This input() algorithm seems only to support tensor operand now.

Actually, the constant(descriptor, bufferView) algorithm in PR #365 should also address this issue.

Right. So this is where conventions about descriptor become important.
I am thinking if it's enough to distinguish between vector and scalar solely on the existence of a dimensions member. But for now let's continue with that working assumption and keep it in mind.

About constant(descriptor, bufferView): the descriptor.dimensions must be defined, otherwise it's an error. There is a polymorphic variant for specifying scalar constant. Should we change that?

Done, please check if correct. Using an assert seemed better choice than just making a note.
Forgot to fix the white space error, such a small change that I amended the same commit and force-pushed.

About constant(descriptor, bufferView): the descriptor.dimensions must be defined, otherwise it's an error.

That's fine. No change needed.

huningxin · 2023-05-10T13:20:01Z

index.bs

        1. If |descriptor| is not an an object that [=implements=] {{MLOperandDescriptor}}, then throw a "{{TypeError}}" {{DOMException}} and stop.
-        1.  If the [=byte length=] of |descriptor| is not supported by the underlying platform, then throw a "{{DataError}}" {{DOMException}} and stop.
-        1. If the [=check dimensions=] steps given |descriptor|.{{MLOperandDescriptor/type}} and |descriptor|.{{MLOperandDescriptor/dimensions}} return `false`, throw a "{{DataError}}" {{DOMException}} and stop.
+        1. If the [=byte length=] of |descriptor| is not supported by the underlying platform, then throw a "{{DataError}}" {{DOMException}} and stop.


"byte length" algorithm also need to handle scalar operand descriptor. Its current definition would break if descriptor.dimensions is undefined.

index.bs

huningxin · 2023-05-10T13:25:12Z

index.bs

+    1. Let |descriptor| be the second argument.
+        1. If |descriptor| is not an an object that [=implements=] {{MLOperandDescriptor}}, then throw a "{{TypeError}}" {{DOMException}} and stop.
+        1.  If the [=byte length=] of |descriptor| is not supported by the underlying platform, then throw a "{{DataError}}" {{DOMException}} and stop.
+        1. If the [=check dimensions=] steps given |descriptor|.{{MLOperandDescriptor/type}} and |descriptor|.{{MLOperandDescriptor/dimensions}} return `false`, throw a "{{DataError}}" {{DOMException}} and stop.


About constant(descriptor, bufferView): the descriptor.dimensions must be defined, otherwise it's an error.

That's fine. No change needed.

huningxin

LGTM, thanks @zolkis !

huningxin · 2023-05-15T01:36:23Z

@zolkis , you may also want to solve the conflicts, squash the commits and edit the commit log before merging. Thanks!

Squashed the following commits: Fix links in algorithm Enclose arg+return description as a note for better clarity Add reference to the platform operand object Handle the scalar case Move byte length check to vector section Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>

also provide SVG source

Reword and refine the text based on review feeedback

Make the text more concise to address review feedback

as discussed in w3c/transitions#491

1. Change `starts` and `sizes` parameters to be sequence of unsigned long. 2. Remove `MLSliceOptions.axes`. Fix webmachinelearning#369

* Draft * More text. * mote text * Address PR feedbacks.

Squashed the following commits: Enclose arg + return descriptions to separate from normative algorithms Change manual link to autolink Use the 'get a copy' reference for buffer source Factor our validating bufferview with descriptor Fix initialization typo Add reference to the platform operand object MLOperand/[[operand]] Lift dependency changes from webmachinelearning#337 Fix typos, remove unneeded dependencies from webmachinelearning#337 Add dependency for buffer validation from webmachinelearning#329 Fix the scalar algorithm / comments Discern between scalar and tensor constant, fix typos Fix review comments. Add stylistic boxes for algorithm steps, informal steps, internal slots Fix typo. Remove stylistic boxes for now. Address reviews comments. Remove MLOperandKind typedef from the IDL. Move note to text. Make the check on kind an assert. Remove the MLOperand/[[kind]] internal slot and creation parameter. Remove back quotes from title Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>

Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>

zolkis · 2023-05-15T06:35:11Z

@huningxin
I rebased, squashed, fixed conflicts, verified with build, then force pushed.
Since there were a lot of other commits in between filing the PR and now, those commits appear here now.
Needed to add one more commit to merge with the main branch.

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

wchao1115 approved these changes Apr 12, 2023

View reviewed changes

anssiko mentioned this pull request Apr 13, 2023

Use modern WebIDL and Infra standard conventions #210

Closed

zolkis changed the title ~~WiP: Add the builder.input() method steps~~ Add the builder.input() method steps Apr 13, 2023

huningxin reviewed May 10, 2023

View reviewed changes

zolkis force-pushed the builder-input-method branch from 563ff94 to d6abd76 Compare May 10, 2023 09:12

huningxin reviewed May 10, 2023

View reviewed changes

huningxin approved these changes May 10, 2023

View reviewed changes

Zoltan Kis and others added 21 commits May 15, 2023 09:19

Update arch diagram to remove WebGL

d98343b

also provide SVG source

Fix native API name

5200c12

Disable echidna in preparation for CR snapshot publication

8d1d87a

Add Status of this document note for Candidate Recommendation

ae6eb2e

Update Status of this document note for Candidate Recommendation

1331089

Reword and refine the text based on review feeedback

Update Status of this document note for Candidate Recommendation

8a28fd9

Make the text more concise to address review feedback

Document CR exit criteria and duration

c4bd2ca

as discussed in w3c/transitions#491

Be less specific on platform API names

cddfdd7

Add link to implementation report

3c5e349

Move CR end deadline to bikeshed metadata

a8a3610

Prepare for automatic CRD publications

cf6f709

Refer to the latest published version of the WebNN API

085caa5

Revert the URL to github.io from w3.org in index.bs

c2f195b

Update newShape with null value for reshape operation

ff3a922

Simplify slice op

908a694

1. Change `starts` and `sizes` parameters to be sequence of unsigned long. 2. Remove `MLSliceOptions.axes`. Fix webmachinelearning#369

Address Jiewei's comments

d46daf9

Add constraint to starting index

0823daf

Additional guidance for contributions (webmachinelearning#381)

39b318e

* Draft * More text. * mote text * Address PR feedbacks.

Fix first-wave models table formatting

c283ebe

Fix a typo of elu sample code

c5f1574

zolkis force-pushed the builder-input-method branch from 1a1a0d4 to c5f1574 Compare May 15, 2023 06:24

Merge main branch into input()

25f8982

Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>

huningxin merged commit 9774d49 into webmachinelearning:main May 16, 2023

github-actions bot added a commit that referenced this pull request May 16, 2023

Merge pull request #364 from zolkis/builder-input-method

33ab296

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

zolkis deleted the builder-input-method branch May 16, 2023 14:53

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the builder.input() method steps#364

Add the builder.input() method steps#364
huningxin merged 23 commits intowebmachinelearning:mainfrom
zolkis:builder-input-method

zolkis commented Mar 22, 2023 •

edited by pr-preview bot

Loading

Uh oh!

zolkis commented Apr 20, 2023

Uh oh!

huningxin May 10, 2023

Uh oh!

huningxin May 10, 2023

Uh oh!

huningxin May 10, 2023

Uh oh!

zolkis May 10, 2023

Uh oh!

zolkis May 10, 2023

Uh oh!

huningxin May 10, 2023

Uh oh!

huningxin May 10, 2023

Uh oh!

Uh oh!

huningxin May 10, 2023

Uh oh!

huningxin left a comment

Uh oh!

huningxin commented May 15, 2023

Uh oh!

zolkis commented May 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Conversation

zolkis commented Mar 22, 2023 • edited by pr-preview bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zolkis commented Apr 20, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huningxin left a comment

Choose a reason for hiding this comment

Uh oh!

huningxin commented May 15, 2023

Uh oh!

zolkis commented May 15, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

zolkis commented Mar 22, 2023 •

edited by pr-preview bot

Loading