Skip to content

Comments

Operator set wave 3#805

Merged
fdwr merged 49 commits intowebmachinelearning:mainfrom
fdwr:opsetWave3
Mar 11, 2025
Merged

Operator set wave 3#805
fdwr merged 49 commits intowebmachinelearning:mainfrom
fdwr:opsetWave3

Conversation

@fdwr
Copy link
Collaborator

@fdwr fdwr commented Jan 16, 2025

Adds the following operators, per #375 (comment) Support for transformers:

Todos

Remaining Either done now, or deferred to separate CR/issue:

API

partial interface MLGraphBuilder
{
    ...
    MLOperand cumulativeSum(MLOperand input, unsigned long axis, optional MLCumulativeSumOptions options = {});
    MLOperand sign(MLOperand input, optional MLOperatorOptions options = {});
    MLOperand tile(MLOperand input, sequence<unsigned long> repetitions, optional MLOperatorOptions options = {});

    // Extends the family beyond the existing gather.
    MLOperand gatherElements(MLOperand input, MLOperand indices, optional MLGatherOptions options = {});
    MLOperand scatterElements(MLOperand input, MLOperand indices, MLOperand updates, optional MLScatterOptions options = {});
    MLOperand gatherND(MLOperand input, MLOperand indices, optional MLOperatorOptions options = {});
    MLOperand scatterND(MLOperand input, MLOperand indices, MLOperand updates, optional MLOperatorOptions options = {});

    MLOperand dequantizeLinear(MLOperand input, MLOperand scale, MLOperand zeroPoint, optional MLOperatorOptions options = {});
    MLOperand quantizeLinear(MLOperand input, MLOperand scale, MLOperand zeroPoint, optional MLOperatorOptions options = {});

    MLOperand logicalAnd(MLOperand a, MLOperand b, optional MLOperatorOptions options = {});
    MLOperand logicalOr(MLOperand a, MLOperand b, optional MLOperatorOptions options = {});
    MLOperand logicalXor(MLOperand a, MLOperand b, optional MLOperatorOptions options = {});
    MLOperand notEqual(MLOperand a, MLOperand b, optional MLOperatorOptions options = {});

    MLOperand reverse(MLOperand input, optional MLReverseOptions options = {});
    MLOperand slice(
        MLOperand input,
        sequence<[EnforceRange] unsigned long> starts,
        sequence<[EnforceRange] unsigned long> sizes,
        optional MLSliceOptions options = {} // Now includes steps
    );
    ...
}
dictionary MLCumulativeSumOptions : MLOperatorOptions
{
    boolean exclusive = false; // Post-sum addition rather than inclusive pre-sum. https://en.wikipedia.org/wiki/Prefix_sum
    boolean reversed = false; // Reverse the summation direction
}

// Already exists for `gather`. Reuse for `gatherElements` too.
dictionary MLGatherOptions : MLOperatorOptions
{
    unsigned long axis = 0;
};

dictionary MLScatterOptions : MLOperatorOptions
{
    unsigned long axis = 0;
};

dictionary MLReverseOptions : MLOperatorOptions
{
    sequence<[EnforceRange] unsigned long> axes;
};

dictionary MLSliceOptions : MLOperatorOptions
{
    sequence<[EnforceRange] unsigned long> strides;
};

Preview | Diff

@inexorabletash
Copy link
Contributor

Another "TODO" - the new ops need "constraints" tables

@fdwr fdwr changed the title Opset wave3 Operator set wave 3 Jan 16, 2025
@inexorabletash
Copy link
Contributor

Note from discussion w/ @a-sully - CoreML has restrictions on the dequantize op that we'll need to think about.

  • scale and bias must be constant
  • the input must be int8 or uint8 (note: CoreML has a different operator for (u)int4, but it requires everything - including the input - to be constant)
  • scale and bias must be either scalars or 1D
  • scale and bias must be the same rank (so, both scalars or both 1D)
  • scale must be the same data type as the output (note: the existing WPTs appear to assert the scale is always float32 (and the description of that test appears to have a typo))
  • scale must be positive

Re-emphasizing that dequantizing (u)int4 in CoreML is extremely limited (input must be const). @mwyrzykowski - any thoughts about how we can handle the proposed ops efficiently?

Copy link
Contributor

@inexorabletash inexorabletash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Initial pass.

@fdwr
Copy link
Collaborator Author

fdwr commented Jan 17, 2025

Another "TODO" - the new ops need "constraints" tables

Added data type tables.

Initial pass.

Thanks - will address more tomorrow after the weekend.

@inexorabletash
Copy link
Contributor

inexorabletash commented Jan 17, 2025

Add "Resolves #779" to the summary, so that issue will get linked to this PR and auto-closed when this merges?

... and "Resolves #773"
... and "Resolves #772"
... and "Resolves #467"
... and "Resolves #93" (I think?)

Maybe #767 too

Copy link
Collaborator Author

@fdwr fdwr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed most feedback (not output shape calculation and blockwise compatible definition).

index.bs Outdated
</details>

### scatterElements ### {#api-mlgraphbuilder-scatterelements}
Scatter values from the updates tensor along an axis according to the indices in place of the input tensor.
Copy link
Collaborator Author

@fdwr fdwr Feb 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 What about "atop" in-place of inplace? Or in-place of a copy of?

Copy link
Contributor

@inexorabletash inexorabletash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another batch of feedback; I didn't make it all the way through the PR though.

…ements examples, fix flatten on edge conditions
Copy link
Contributor

@inexorabletash inexorabletash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link
Member

@anssiko anssiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🌊 Huge thank you to @fdwr for this significant op set update and @inexorabletash @huningxin, other participants for contributions and comments, in total close to 150 over 200!

As discussed, we'll periodically seek TAG review for significant changes to the spec. This spec update complemented with a demonstration of implementation experience across multiple backends and OSes will be brought to the TAG's attention alongside other significant changes.

Copy link
Contributor

@inexorabletash inexorabletash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, noted a few more places where an arg reference should be linkified rather than just styled.

Copy link
Collaborator Author

@fdwr fdwr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That should address all of Ningxin and Joshua's last feedback. 🤞

Copy link
Contributor

@huningxin huningxin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@fdwr
Copy link
Collaborator Author

fdwr commented Mar 11, 2025

@inexorabletash Want to take one more pass?
If good, will merge.

@inexorabletash
Copy link
Contributor

I've been following along, still LGTM. Merge away!

@fdwr fdwr merged commit 6e19654 into webmachinelearning:main Mar 11, 2025
2 checks passed
github-actions bot added a commit that referenced this pull request Mar 11, 2025
SHA: 6e19654
Reason: push, by fdwr

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
aarongable pushed a commit to chromium/chromium that referenced this pull request Jul 24, 2025
This CL implements the WebNN spec change [1] that requires the scale
and zeroPoint for quantizeLinear and dequantizeLinear ops have the
the same rank as input.

[1]: webmachinelearning/webnn#805 (comment)

Bug: 396176047
Change-Id: Ia310e0c254ad90967b4e3577e54498fa28532f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6773953
Commit-Queue: ningxin hu <[email protected]>
Reviewed-by: Phillis Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1491196}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 24, 2025
This CL implements the WebNN spec change [1] that requires the scale
and zeroPoint for quantizeLinear and dequantizeLinear ops have the
the same rank as input.

[1]: webmachinelearning/webnn#805 (comment)

Bug: 396176047
Change-Id: Ia310e0c254ad90967b4e3577e54498fa28532f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6773953
Commit-Queue: ningxin hu <[email protected]>
Reviewed-by: Phillis Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1491196}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jul 24, 2025
This CL implements the WebNN spec change [1] that requires the scale
and zeroPoint for quantizeLinear and dequantizeLinear ops have the
the same rank as input.

[1]: webmachinelearning/webnn#805 (comment)

Bug: 396176047
Change-Id: Ia310e0c254ad90967b4e3577e54498fa28532f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6773953
Commit-Queue: ningxin hu <[email protected]>
Reviewed-by: Phillis Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1491196}
lando-prod-mozilla bot pushed a commit to mozilla-firefox/firefox that referenced this pull request Jul 28, 2025
…s equal to input rank, a=testonly

Automatic update from web-platform-tests
WebNN: Ensure scale and zeroPoint rank is equal to input rank

This CL implements the WebNN spec change [1] that requires the scale
and zeroPoint for quantizeLinear and dequantizeLinear ops have the
the same rank as input.

[1]: webmachinelearning/webnn#805 (comment)

Bug: 396176047
Change-Id: Ia310e0c254ad90967b4e3577e54498fa28532f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6773953
Commit-Queue: ningxin hu <[email protected]>
Reviewed-by: Phillis Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1491196}

--

wpt-commits: 03b1ef622ac6f93394a64d4213d80a0e8881254f
wpt-pr: 53947
foolip pushed a commit to web-platform-tests/wpt that referenced this pull request Aug 13, 2025
This CL implements the WebNN spec change [1] that requires the scale
and zeroPoint for quantizeLinear and dequantizeLinear ops have the
the same rank as input.

[1]: webmachinelearning/webnn#805 (comment)

Bug: 396176047
Change-Id: Ia310e0c254ad90967b4e3577e54498fa28532f84
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6773953
Commit-Queue: ningxin hu <[email protected]>
Reviewed-by: Phillis Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1491196}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants