Skip to content

[WebNN EP] Remove workaround for dynamic shape#17644

Merged
fdwr merged 3 commits intomicrosoft:mainfrom
Honry:fix-dynamic-shape
Sep 22, 2023
Merged

[WebNN EP] Remove workaround for dynamic shape#17644
fdwr merged 3 commits intomicrosoft:mainfrom
Honry:fix-dynamic-shape

Conversation

@Honry
Copy link
Contributor

@Honry Honry commented Sep 21, 2023

As now we have the FreeDimensionOverrides option to support dynamic shape, we can remove the previous
workaround.

As now we have the FreeDimensionOverrides option to
support dynamic shape, we can remove the previous
workaround.
@Honry
Copy link
Contributor Author

Honry commented Sep 21, 2023

@fdwr, @guschmue, PTAL, thanks!

Copy link
Contributor

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

Nits, else LGTM.

@Honry
Copy link
Contributor Author

Honry commented Sep 21, 2023

Thanks @fdwr, nits addressed.

fdwr
fdwr previously approved these changes Sep 21, 2023
Copy link
Contributor

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

👍

@fdwr
Copy link
Contributor

fdwr commented Sep 21, 2023

/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 Sep 21, 2023

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

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@fdwr
Copy link
Contributor

fdwr commented Sep 21, 2023

@Honry Hmm, the linter is complaining, but I can't tell what it's saying because the proposed insertion is identical to the old line 🙃. It seems the tool is not the brightest bulb in it's messaging, but I'm guessing it's complaining that there are no spaces before the comment:

image

@Honry
Copy link
Contributor Author

Honry commented Sep 22, 2023

@Honry Hmm, the linter is complaining, but I can't tell what it's saying because the proposed insertion is identical to the old line 🙃. It seems the tool is not the brightest bulb in it's messaging, but I'm guessing it's complaining that there are no spaces before the comment:

image

:( Lines should be <= 120 characters long [whitespace/line_length] [2], I will fix that.

Copy link
Contributor

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

👍

@fdwr
Copy link
Contributor

fdwr commented Sep 22, 2023

/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 Sep 22, 2023

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

@fdwr
Copy link
Contributor

fdwr commented Sep 22, 2023

/azp run ONNX Runtime Web CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 9 pipeline(s).

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@fdwr
Copy link
Contributor

fdwr commented Sep 22, 2023

/azp run Windows x64 QNN CI Pipeline

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@microsoft microsoft deleted a comment from azure-pipelines bot Sep 22, 2023
@fdwr fdwr merged commit ce287a4 into microsoft:main Sep 22, 2023
kleiti pushed a commit to kleiti/onnxruntime that referenced this pull request Mar 22, 2024
As now we have the FreeDimensionOverrides option to support dynamic
shape, we can remove the previous
workaround.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants