-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[WebNN EP] Support Einsum op #19558
base: main
Are you sure you want to change the base?
[WebNN EP] Support Einsum op #19558
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @peishenyan, some comments.
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
Hi, all. Last week, I implemented Einsum based on the DML execution provider's implementation, which only support a few conditions. However, Einsum('bhwc,hkc->bhwk', A, B) cannot be handled in ``Segment Anything Encoder''. Now, according to Einsum Formula in ML Operator Formulas, I implement pair-wise operand processing for handling two inputs and kept the previously implemented single operand processing (Identity, Transpose and ReduceSum.) |
Since WebNN does not support Triangular op so far and Diagonal depends on triangular, WebNN will fallback when the Einsum op is used for Diagonal operation. One more thing, in Segment Anything Encoder model, the output shape of Einsum is not defined, which is the input of some Unsqueeze operation, while WebNN does not support dynamic shape. We need additional work to solve this problem in another work branch. |
I will leave for about one week for my research paper submission. Responses may be delayed. |
Thanks @peishenyan! @guschmue, @fdwr, PTAL thanks!
@guschmue, @fdwr, the output shape of So my question is, is that possible to calculate and add its output shape info to the graph during ORT Web graph optimization? |
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
Are you saying that ORT is not doing shape inference properly for EinSum? The DML EP (which also needs constant shapes before DML CompileGraph is called) registers its own shape inference function for EinSum here onnxruntime/onnxruntime/core/providers/dml/OperatorAuthorHelper/OperatorHelper.cpp Lines 1632 to 1641 in 1e69b61
👍 Lisha is working on it (and I drew some whiteboard diagrams on decomposing it for DML feature level 4). It should be available soon.
Thanks for creating this - hope your research paper goes well. |
Right, ORT doesn't do shape inference for
Interesting, how and when does DML EP register the shape info to the graph? |
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Peishen. I'm still trying to figure out PairwiseOperandProcess
(some more comments/visuals/examples could help, especially future generations), but I reviewed the rest of it.
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
Hi @fdwr , I have addressed most of the comments, but the code is not ready for final review now. |
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline |
/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models |
Azure Pipelines successfully started running 9 pipeline(s). |
Azure Pipelines successfully started running 10 pipeline(s). |
Any test can cover the op in WebNN EP? |
@tianleiwu, good point! @peishenyan has ever tested the following unit tests, but which are all fp64 data type that currently is not supported by WebNN. Is that possible to change the data type to fp32? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY Peishen. Some more thoughts. I'll make a few trivial edits directly (little typos), and for the renaming, we can go ahead and submit as-is if you can follow up in a small CR to make some of the local variable names cleaner.
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
|
||
std::vector<uint32_t> sequence_o(output_indices.size()); | ||
std::iota(sequence_o.begin(), sequence_o.end(), 0); | ||
if (v != sequence_o) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (output_permutation != sequence_o) {
We should use a clearer name than just "v". Does output_permutation
make sense here? With the exception of really common cases (like i
for loop counters and x
,y
,z
for coordinates), single letter variable names are best avoided. Similarly s
and t
are unclear what they stand for?
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
onnxruntime/core/providers/webnn/builders/impl/einsum_op_builder.cc
Outdated
Show resolved
Hide resolved
/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline |
/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models |
Azure Pipelines successfully started running 10 pipeline(s). |
Azure Pipelines successfully started running 9 pipeline(s). |
/azp run Linux Android Emulator QNN CI Pipeline |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline |
/azp run Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline,Big Models |
Azure Pipelines successfully started running 10 pipeline(s). |
Azure Pipelines successfully started running 9 pipeline(s). |
/azp run Linux Android Emulator QNN CI Pipeline |
Azure Pipelines successfully started running 1 pipeline(s). |
Hi @fdwr, long time no see. Really appreciate your help. I will address the comments today~ |
Hi all, I have addressed the comments in the latest commit. |
Thanks for confirmation. Ok, will defer merging. So then for any models that need the shape inference logic (like the Segment Anything encoder https://github.com/microsoft/webnn-developer-preview/blob/main/demos/segment-anything/index.js#L739), we'll need a custom build of ORT and copy the .wasm files onto the server. |
525893c
to
c5ad5c3
Compare
Relevant! #21897 |
Address comments register einsum op add logs for einsum Fully implement Einsum op for WebNN EP Address comments address comments fix bugs fix bugs add some comments and fix einsum_type error lint and apply the fixes address comments add TODO and fix identity case remove useless code fix permutation error Apply suggestions from code review Typos and minor declaration. address comments add datatype check and modify IsOpSupported check add diagonal and fix some bugs
c5ad5c3
to
af7d5f7
Compare
/azp run Windows ARM64 QNN CI Pipeline,Windows x64 QNN CI Pipeline,Windows CPU CI Pipeline,Windows GPU CUDA CI Pipeline,Windows GPU DML CI Pipeline,Windows GPU Doc Gen CI Pipeline,Windows GPU TensorRT CI Pipeline,ONNX Runtime Web CI Pipeline,Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline |
/azp run Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,orttraining-amd-gpu-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,onnxruntime-binary-size-checks-ci-pipeline |
/azp run Big Models,Linux Android Emulator QNN CI Pipeline,Android CI Pipeline,iOS CI Pipeline,ONNX Runtime React Native CI Pipeline |
Azure Pipelines successfully started running 5 pipeline(s). |
Azure Pipelines successfully started running 8 pipeline(s). |
Azure Pipelines successfully started running 10 pipeline(s). |
Hi @fdwr, do you have any other comment? Could we merge this branch? |
Adds support for einsum via WebNN matmul, transpose, reshape, reducesum, identity and element-wise binary ops.