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

keepDimensions of reduction ops #465

Closed
lisa0314 opened this issue Sep 22, 2023 · 3 comments · Fixed by #648
Closed

keepDimensions of reduction ops #465

lisa0314 opened this issue Sep 22, 2023 · 3 comments · Fixed by #648
Assignees

Comments

@lisa0314
Copy link

XNNPACK can't support keepDimensions = true for ReduceMean.
This issue was raised by @huningxin in WebNN Chromium CL review. Thanks Ningxin!

@fdwr
Copy link
Collaborator

fdwr commented Sep 28, 2023

keepDimensions just controls the graph builder's output dimension description and does not change how tensor elements are read or written (the output memory is identical either way). So a backend supporting keepDimensions or not itself doesn't matter for the WebNN caller, since it's just a matter of updating the tensor description in the backend - a nop reshape.

For contrast, the DirectML backend only supports the reverse, keepDimensions == true, but keepDimensions == false still works fine because the lightweight tensor dimension is massaged before calling DirectML. In both cases below, DML_OPERATOR_REDUCE receives [2,1,4] for the output tensor desc, because the backend inserts the dummy 1's (note there is no actual "reshape" here, but conceptually it is).

  1. input sizes = [2,3,4], keep dimensions = true, axis = 1, output sizes = [2,1,4]. DML sees output [2,1,4]
  2. input sizes = [2,3,4], keep dimensions = false, axis = 1, output sizes = [2, 4]. DML sees output [2,1,4]

Conversely, XNNPack should see:

  1. input sizes = [2,3,4], keep dimensions = true, axis = 1, output sizes = [2,1,4]. XNNPack sees output [2,4]
  2. input sizes = [2,3,4], keep dimensions = false, axis = 1, output sizes = [2, 4]. XNNPack sees output [2,4]

@inexorabletash
Copy link
Member

Based on @fdwr's comment, would this be resolved by adding a non-normative note with hints to implementers?

@huningxin
Copy link
Contributor

XNNPACK now supports keepDimensions = true for ReduceMean: google/XNNPACK@5ecf0769c54cd224bd0026fe2c

@inexorabletash inexorabletash self-assigned this Apr 18, 2024
inexorabletash added a commit to inexorabletash/webnn that referenced this issue Apr 18, 2024
Add a note based on @fdwr's comment: if the backend doesn't support a
keepDimensions option, the implementation can insert a no-op reshape
to keep or drop the reduced dimension(s) as needed.

Fixes webmachinelearning#465
@fdwr fdwr closed this as completed in #648 Apr 19, 2024
fdwr pushed a commit that referenced this issue Apr 19, 2024
Add a note based on @fdwr's comment: if the backend doesn't support a
keepDimensions option, the implementation can insert a no-op reshape
to keep or drop the reduced dimension(s) as needed.

Fixes #465
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.

5 participants