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.

Sign up for GitHub

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

Jump to bottom

[WebNN EP] Add outputDataType option for the ArgMax/ArgMin ops #21385

Merged
merged 1 commit into from
Jul 22, 2024
Merged

[WebNN EP] Add outputDataType option for the ArgMax/ArgMin ops #21385

merged 1 commit into from
Jul 22, 2024

Conversation

Honry
Copy link
Contributor

@Honry Honry commented Jul 17, 2024 •
edited by fdwr
Loading

Description

WebNN spec introduces a new option: outputDataType to argMax and argMin ops, it's default value is int32, we should explicitly set it to int64 for WebNN EP.

Spec CR: "Add outputDataType to argmin/argmax" webmachinelearning/webnn#730

@Honry
Copy link
Contributor Author

Honry commented Jul 17, 2024

@fdwr, @guschmue, PTAL, thanks!

cc/ @huningxin

@huningxin huningxin mentioned this pull request Jul 17, 2024
Closed
@fdwr fdwr added the ep:WebNN WebNN execution provider label Jul 17, 2024
fdwr
fdwr previously approved these changes Jul 17, 2024
View reviewed changes
@fdwr
Copy link
Contributor

fdwr commented Jul 17, 2024

@Honry Thank you for updating it promptly to match the Chromium implementation.

@fdwr
Copy link
Contributor

fdwr commented Jul 17, 2024

/azp run ONNX Runtime Web CI Pipeline,Windows GPU CI Pipeline,Linux Android Emulator QNN CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 17, 2024

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 17, 2024

/azp run Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,Windows x64 QNN CI Pipeline,Big Models

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@fdwr
Copy link
Contributor

fdwr commented Jul 18, 2024

/azp run Windows GPU CI Pipeline

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fdwr fdwr mentioned this pull request Jul 18, 2024
Closed
huningxin
huningxin approved these changes Jul 18, 2024
View reviewed changes
Copy link

@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 with a comment

onnxruntime/core/providers/webnn/builders/impl/argmax_min_op_builder.cc Show resolved Hide resolved
@fdwr
Copy link
Contributor

fdwr commented Jul 18, 2024

/azp run ONNX Runtime Web CI Pipeline,Windows GPU CI Pipeline,Linux Android Emulator QNN CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 18, 2024

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 18, 2024

/azp run Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,Windows x64 QNN CI Pipeline,Big Models

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024

/azp run Windows GPU TensorRT CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline

fdwr
fdwr previously approved these changes Jul 19, 2024
View reviewed changes
@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024 •
edited
Loading

@Honry I don't know why the GPU tests keep getting canceled. Maybe there is a change in main we should merge into this branch?

@Honry
Copy link
Contributor Author

Honry commented Jul 19, 2024

@Honry I don't know why the GPU tests keep getting canceled. Maybe there is a change in main we should merge into this branch?

I will do a rebase.

@Honry Honry force-pushed the argmaxmin-outputDataType branch from 6920447 to ca89be0 Compare July 19, 2024 02:34
@Honry
Copy link
Contributor Author

Honry commented Jul 19, 2024

@Honry I don't know why the GPU tests keep getting canceled. Maybe there is a change in main we should merge into this branch?

I will do a rebase.

@fdwr, done, please help retrigger the CI.

@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024

/azp run Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,Windows x64 QNN CI Pipeline,Big Models

@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline

@fdwr
Copy link
Contributor

fdwr commented Jul 19, 2024

/azp run ONNX Runtime Web CI Pipeline,Windows GPU CI Pipeline,Linux Android Emulator QNN CI Pipeline

1 similar comment
@guschmue
Copy link
Contributor

/azp run ONNX Runtime Web CI Pipeline,Windows GPU CI Pipeline,Linux Android Emulator QNN CI Pipeline

@guschmue
Copy link
Contributor

/azp run Linux CPU CI Pipeline,Linux CPU Minimal Build E2E CI Pipeline,Linux GPU CI Pipeline,Linux GPU TensorRT CI Pipeline,Linux OpenVINO CI Pipeline,Linux QNN CI Pipeline,MacOS CI Pipeline,Windows ARM64 QNN CI Pipeline,Windows CPU CI Pipeline

@guschmue
Copy link
Contributor

/azp run Windows GPU TensorRT CI Pipeline,onnxruntime-binary-size-checks-ci-pipeline,orttraining-linux-ci-pipeline,orttraining-linux-gpu-ci-pipeline,orttraining-ortmodule-distributed,Windows x64 QNN CI Pipeline,Big Models

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 7 pipeline(s).

@fdwr
Copy link
Contributor

fdwr commented Jul 20, 2024

/azp run Windows CPU CI Pipeline, Windows GPU CI Pipeline

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@fdwr fdwr merged commit 17e9ea6 into microsoft:main Jul 22, 2024
75 checks passed
@ibelem ibelem mentioned this pull request Jul 23, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fdwr fdwr fdwr approved these changes

@huningxin huningxin huningxin approved these changes

Assignees
No one assigned
Labels
ep:WebNN WebNN execution provider
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Honry @fdwr @guschmue @huningxin