Skip to content

add minimum data types and rank range for operations#910

Open
BruceDai wants to merge 16 commits into
webmachinelearning:mainfrom
BruceDai:minimum_datatype_rankrange
Open

add minimum data types and rank range for operations#910
BruceDai wants to merge 16 commits into
webmachinelearning:mainfrom
BruceDai:minimum_datatype_rankrange

Conversation

@BruceDai
Copy link
Copy Markdown
Contributor

@BruceDai BruceDai commented Dec 10, 2025 •
edited by pr-preview Bot
Loading

This PR is to fix #896.
@huningxin @fdwr @reillyeon PTAL, thanks!

Preview | Diff

@BruceDai BruceDai changed the title add minimum data types and rank range for operations [wip]add minimum data types and rank range for operations Dec 11, 2025
BruceDai
BruceDai commented Dec 12, 2025
View reviewed changes
Comment thread index.bs
BruceDai
BruceDai commented Dec 12, 2025
View reviewed changes
Comment thread index.bs
BruceDai
BruceDai commented Dec 12, 2025
View reviewed changes
Comment thread index.bs Outdated
BruceDai
BruceDai commented Dec 12, 2025
View reviewed changes
Comment thread index.bs Outdated
@BruceDai BruceDai changed the title [wip]add minimum data types and rank range for operations add minimum data types and rank range for operations Dec 12, 2025
@fdwr
Copy link
Copy Markdown
Collaborator

fdwr commented Dec 12, 2025 •
edited
Loading

I checked the unchanged Line 8987, it seems correct by using current 8 indents for to match front

of also 8 indents. Any suggestions? Thanks!

@BruceDai : Remerge with main. My PR adds an extra blank line that pacifies the build error (and yes, that line was already correct, and nothing changed there for months, but bikeshed changed).

BruceDai
BruceDai commented Dec 12, 2025
View reviewed changes
Comment thread index.bs
reillyeon
reillyeon suggested changes Dec 13, 2025
View reviewed changes
Comment thread index.bs Outdated
reillyeon
reillyeon approved these changes Dec 16, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@reillyeon reillyeon left a comment

Choose a reason for hiding this comment

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

General wording looks good. I have not validated the specific operator limits specified.

Comment thread index.bs Outdated
Comment thread index.bs Outdated
fdwr
fdwr reviewed Dec 16, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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.

👀 Thanks for the updates Bruce - I'm sure they were tedious :b.

Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs
Comment thread index.bs
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}</td>
<td>[=/any rank|N=]</td>
<td>0 to 4</td>
Copy link
Copy Markdown
Collaborator

@fdwr fdwr Dec 16, 2025

Choose a reason for hiding this comment

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

Which ML API is the outlier for pow being 0 to 4 instead of 0 to 5? It will be problematic for callers if mul/div/pow have inconsistent ranks from each other, as certain substitution operations might expect that they can safely chain these or switch from one to another (e.g. mul(a,a) == pow(a,2)) without surprise. I propose the outlier API fix this in their Chromium backend with flanking reshapes (flatten to 1D if > 4D, pow, then back to the original shape). Then pow can be remerged back with the rest above.

Copy link
Copy Markdown
Contributor Author

@BruceDai BruceDai Dec 17, 2025

Choose a reason for hiding this comment

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

Which ML API is the outlier for pow being 0 to 4 instead of 0 to 5?

It's limited by TFLite backend.
Please refer to https://source.chromium.org/chromium/chromium/src/+/main:services/webnn/tflite/graph_builder_tflite.cc;l=568

       // Limited to 4D when broadcasting is required:
       // https://source.chromium.org/chromium/chromium/src/+/main:third_party/tflite/src/tensorflow/lite/kernels/pow.cc
       /*pow_input=*/{kFloat16To32AndInt32, SupportedRanks::UpTo(4)},

Copy link
Copy Markdown
Collaborator

@fdwr fdwr Dec 23, 2025

Choose a reason for hiding this comment

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

Can we open a TFLite issue? Alternately we open a Chromium issue to {reshape, expand if needed, pow} until TFLite can address it.

Copy link
Copy Markdown
Contributor Author

@BruceDai BruceDai Dec 23, 2025

Choose a reason for hiding this comment

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

Filed a Chromium issue - https://issues.chromium.org/issues/471017637.

Copy link
Copy Markdown
Contributor

@huningxin huningxin May 12, 2026

Choose a reason for hiding this comment

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

Filed a Chromium issue - https://issues.chromium.org/issues/471017637.

@BruceDai , what's your plan for fixing this issue?

Copy link
Copy Markdown
Contributor

@huningxin huningxin May 28, 2026

Choose a reason for hiding this comment

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

Filed a Chromium issue - https://issues.chromium.org/issues/471017637.

@BruceDai , what's your plan for fixing this issue?

@BruceDai , I guess this issue can be fixed in a similar way of 7848774: webnn: align logical op rank limits at 5 on the TFLite backend?

Comment thread index.bs
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs
Comment thread index.bs
huningxin
huningxin reviewed Dec 17, 2025
View reviewed changes
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs Outdated
<tr>
<td>{{input}}</td>
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}</td>
Copy link
Copy Markdown
Contributor

@huningxin huningxin Dec 17, 2025

Choose a reason for hiding this comment

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

ORT doesn't support int32

Copy link
Copy Markdown
Collaborator

@fdwr fdwr Dec 18, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🤔 An ONNX BitCast operator could help with cases of pure data movement (since no actual math happens) like this one when the operator supports a different data type of the same bit size (sizeof(int32) == sizeof(uint32) == sizeof(float32)). For example, we could call BitCast(int32input, type=float32) -> Triangular -> BitCast(input, type=int32). I'll comment on that ONNX PR.

(update) BitCast operator is pending: onnx/onnx#7527

Copy link
Copy Markdown
Collaborator

@fdwr fdwr Apr 13, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

(update) BitCast is in ONNX https://onnx.ai/onnx/operators/onnx__BitCast.html, now enabling callers of the ORT backend to support all data types of the same bit size for data movement operators, so long as the EP supports BitCast and at least one data type of a given bit size (e.g. supporting int16 with Gather also enables uint16, float16, bfloat16...).

@fdwr fdwr mentioned this pull request Dec 18, 2025
Closed
@BruceDai
Copy link
Copy Markdown
Contributor Author

BruceDai commented Dec 22, 2025

Thanks @huningxin and @fdwr for review and identifications. 👍
I submitted a new commit to address your comments. Please take another look, thanks!☕

@BruceDai BruceDai force-pushed the minimum_datatype_rankrange branch 2 times, most recently from 949a42f to b6b300f Compare January 16, 2026 09:01
fdwr
fdwr approved these changes Mar 11, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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.

👍

@BruceDai BruceDai force-pushed the minimum_datatype_rankrange branch from b6b300f to 3f29bff Compare April 13, 2026 13:25
mwyrzykowski
mwyrzykowski approved these changes May 7, 2026
View reviewed changes
Copy link
Copy Markdown

@mwyrzykowski mwyrzykowski left a comment

Choose a reason for hiding this comment

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

Conflict with the WG meeting, but ✅

fdwr
fdwr approved these changes May 12, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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.

👍

huningxin
huningxin reviewed May 12, 2026
View reviewed changes
Comment thread index.bs Outdated
Comment thread index.bs Outdated
Comment thread index.bs
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}</td>
<td>[=/any rank|N=]</td>
<td>0 to 4</td>
Copy link
Copy Markdown
Contributor

@huningxin huningxin May 12, 2026

Choose a reason for hiding this comment

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

Filed a Chromium issue - https://issues.chromium.org/issues/471017637.

@BruceDai , what's your plan for fixing this issue?

Comment thread index.bs
@BruceDai
Copy link
Copy Markdown
Contributor Author

BruceDai commented May 14, 2026 •
edited
Loading

@BruceDai , what's your plan for fixing this issue? Generalizing them to 0 to 5D with backend reshapes sounds good to me.

@huningxin Thanks for your kindly reminder! Please review the fixing CL-7848774 .

@BruceDai
Copy link
Copy Markdown
Contributor Author

BruceDai commented May 22, 2026

Thanks @reillyeon @huningxin and @fdwr for reviewing and approving CL-7848774, now it has been landed.
Please take another review the new commit. Thanks! ☕

fdwr
fdwr approved these changes May 22, 2026
View reviewed changes
Copy link
Copy Markdown
Collaborator

@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.

👍 Thank you for enabling this.

huningxin
huningxin reviewed May 28, 2026
View reviewed changes
Comment thread index.bs
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}</td>
<td>[=/any rank|N=]</td>
<td>0 to 4</td>
Copy link
Copy Markdown
Contributor

@huningxin huningxin May 28, 2026

Choose a reason for hiding this comment

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

Filed a Chromium issue - https://issues.chromium.org/issues/471017637.

@BruceDai , what's your plan for fixing this issue?

@BruceDai , I guess this issue can be fixed in a similar way of 7848774: webnn: align logical op rank limits at 5 on the TFLite backend?

Comment thread index.bs
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}, {{MLOperandDataType/"int32"}}</td>
<td>[=/any rank|N=]</td>
<td>0 to 4</td>
Copy link
Copy Markdown
Contributor

@huningxin huningxin May 28, 2026

Choose a reason for hiding this comment

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

Should we also generalize them to 0 to 5D? I already see a CL relies on 5D lesser in TFLite backend: webnn: Support >4D indices for gather/gatherND/scatterND

Comment thread index.bs
<td>[=/any data type|any=]</td>
<td>{{MLOperandDataType/"float32"}}, {{MLOperandDataType/"float16"}}</td>
<td>[=/any rank|N=]</td>
<td>0 to 4</td>
Copy link
Copy Markdown
Contributor

@huningxin huningxin May 28, 2026

Choose a reason for hiding this comment

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

Do the same? Then we will have all logical operators supporting 0 to 5D.

Copy link
Copy Markdown
Collaborator

@fdwr fdwr May 28, 2026

Choose a reason for hiding this comment

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

Then we will have all logical operators supporting 0 to 5D.

Hmm, I wonder how far we are from all elementwise ops ranging 0 through 5?

Copy link
Copy Markdown
Contributor

@huningxin huningxin May 28, 2026

Choose a reason for hiding this comment

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

I guess two or three CLs for Chromium TFLite backend?

  1. pow and logical binary ops
  2. isNan/isInfinite

7848774: webnn: align logical op rank limits at 5 on the TFLite backend already landed necessary helpers.

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

Reviewers

@huningxin huningxin huningxin left review comments

@fdwr fdwr fdwr approved these changes

+2 more reviewers

@reillyeon reillyeon reillyeon approved these changes

@mwyrzykowski mwyrzykowski mwyrzykowski approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add minimum data type set and rank range for input, constant, output and each operator into Spec

5 participants

@BruceDai @fdwr @reillyeon @huningxin @mwyrzykowski