[ONNX] Fix circular padding to support dynamic axes by ilyasher · Pull Request #95647 · pytorch/pytorch

ilyasher · 2023-02-27T20:58:46Z

This commit fixes a bug where the ONNX exporter for circular padding queried the input tensor shape in order to get the correct 'end' index for a slice node. This doesn't work when the axis in question is has dynamic size. The commit fixes this by setting the 'end' index to INT_MAX, which is the recommended way of slicing to the end of a dimension with unknown size per ONNX spec.

See https://onnx.ai/onnx/operators/onnx__Slice.html

Also adds a regression test.

pytorch-bot · 2023-02-27T20:58:49Z

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95647

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit f877b1d:
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

linux-foundation-easycla · 2023-02-27T20:58:50Z

The committers listed above are authorized under a signed CLA.

✅ login: ilyasher / name: Ilya Sherstyuk (05de9063eacdc11ba1e436dc2052a8ed66a8534e, fcc854db62dce16c777e907450b2ee5e0eb7ee4e)

BowenBao

LGTM, thanks for your contribution! You need to sign EasyCLA for me to merge it.

BowenBao · 2023-02-28T17:28:17Z

@pytorchbot rebase

pytorchmergebot · 2023-02-28T17:32:20Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2023-02-28T17:32:26Z

Successfully rebased dev-ilyasher-onnx-circular-pad-dyn-axes onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout dev-ilyasher-onnx-circular-pad-dyn-axes && git pull --rebase)

ilyasher · 2023-02-28T18:44:37Z

LGTM, thanks for your contribution! You need to sign EasyCLA for me to merge it.

Thanks, I'm trying to get that sorted out right now.

test/onnx/test_pytorch_onnx_onnxruntime.py

ilyasher · 2023-03-01T22:06:55Z

/easycla

BowenBao · 2023-03-01T23:08:59Z

The committers listed above are authorized under a signed CLA.

✅ login: ilyasher / name: Ilya Sherstyuk (32e2414, f877b1d)

ilyasher · 2023-03-01T23:45:54Z

I wonder if it's because the other commit has a different author email address.

Good thinking, I will complete the CLA with my other email.

ilyasher · 2023-03-01T23:46:01Z

/easycla

BowenBao · 2023-03-02T00:16:52Z

@pytorchbot rebase

pytorchmergebot · 2023-03-02T00:19:08Z

@pytorchbot successfully started a rebase job. Check the current status here

pytorchmergebot · 2023-03-02T00:19:15Z

Successfully rebased dev-ilyasher-onnx-circular-pad-dyn-axes onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout dev-ilyasher-onnx-circular-pad-dyn-axes && git pull --rebase)

ilyasher · 2023-03-02T00:32:04Z

The commit shows my email as
Author: Ilya Sherstyuk [email protected]

I will fix the commit with the correct email and force-push

ilyasher · 2023-03-02T00:34:44Z

/easycla

ilyasher · 2023-03-08T22:44:03Z

/easycla

ilyasher · 2023-03-08T22:45:21Z

@pytorchbot rebase

pytorch-bot · 2023-03-08T22:45:25Z

You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra.

ilyasher · 2023-03-08T22:46:44Z

@BowenBao I finally got the CLA approved, what else do I need to do to merge this PR? Thanks for all your help.

BowenBao · 2023-03-08T23:48:01Z

@pytorchbot rebase

pytorchmergebot · 2023-03-08T23:49:57Z

@pytorchbot successfully started a rebase job. Check the current status here

This commit fixes a bug where the ONNX exporter for circular padding queried the input tensor shape in order to get the correct 'end' index for a slice node. This doesn't work when the axis in question is has dynamic size. The commit fixes this by setting the 'end' index to INT_MAX, which is the recommended way of slicing to the end of a dimension with unknown size per ONNX spec.

Co-authored-by: Bowen Bao <[email protected]>

pytorchmergebot · 2023-03-08T23:50:03Z

Successfully rebased dev-ilyasher-onnx-circular-pad-dyn-axes onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout dev-ilyasher-onnx-circular-pad-dyn-axes && git pull --rebase)

BowenBao · 2023-03-08T23:58:22Z

@pytorchbot merge -g

pytorchmergebot · 2023-03-09T00:00:14Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

pytorchmergebot · 2023-03-09T00:30:36Z

Merge failed

Reason: Not merging any PRs at the moment because there is a merge blocking https://github.com/pytorch/pytorch/labels/ci:%20sev issue open at:
#96371

Details for Dev Infra team

Raised by workflow job

BowenBao · 2023-03-10T00:27:35Z

@pytorchbot merge -g

pytorchmergebot · 2023-03-10T00:29:28Z

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging

Check the merge workflow status
here

This commit fixes a bug where the ONNX exporter for circular padding queried the input tensor shape in order to get the correct 'end' index for a slice node. This doesn't work when the axis in question is has dynamic size. The commit fixes this by setting the 'end' index to INT_MAX, which is the recommended way of slicing to the end of a dimension with unknown size per ONNX spec. See https://onnx.ai/onnx/operators/onnx__Slice.html Also adds a regression test. Pull Request resolved: pytorch/pytorch#95647 Approved by: https://github.com/BowenBao

ilyasher requested review from BowenBao and abock as code owners February 27, 2023 20:58

pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Feb 27, 2023

pytorchbot added the open source label Feb 27, 2023

H-Huang added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Feb 28, 2023

BowenBao added the module: onnx Related to torch.onnx label Feb 28, 2023

BowenBao approved these changes Feb 28, 2023

View reviewed changes

pytorchmergebot force-pushed the dev-ilyasher-onnx-circular-pad-dyn-axes branch from 39894d2 to d72682b Compare February 28, 2023 17:32

BowenBao reviewed Mar 1, 2023

View reviewed changes

test/onnx/test_pytorch_onnx_onnxruntime.py Outdated Show resolved Hide resolved

pytorchmergebot force-pushed the dev-ilyasher-onnx-circular-pad-dyn-axes branch from a69e3ea to 5f7806e Compare March 2, 2023 00:19

ilyasher force-pushed the dev-ilyasher-onnx-circular-pad-dyn-axes branch from 5f7806e to fcc854d Compare March 2, 2023 00:33

ilyasher and others added 2 commits March 8, 2023 23:50

Fix lint

f877b1d

Co-authored-by: Bowen Bao <[email protected]>

pytorchmergebot force-pushed the dev-ilyasher-onnx-circular-pad-dyn-axes branch from fcc854d to f877b1d Compare March 8, 2023 23:50

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Mar 8, 2023

pytorchmergebot added the Merged label Mar 10, 2023

pytorchmergebot closed this in 6154be1 Mar 10, 2023

ilyasher deleted the dev-ilyasher-onnx-circular-pad-dyn-axes branch May 1, 2023 21:03

Conversation

ilyasher commented Feb 27, 2023

Uh oh!

pytorch-bot bot commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/95647

✅ No Failures

Uh oh!

linux-foundation-easycla bot commented Feb 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

BowenBao left a comment

Choose a reason for hiding this comment

Uh oh!

BowenBao commented Feb 28, 2023

Uh oh!

pytorchmergebot commented Feb 28, 2023

Uh oh!

pytorchmergebot commented Feb 28, 2023

Uh oh!

ilyasher commented Feb 28, 2023

Uh oh!

Uh oh!

ilyasher commented Mar 1, 2023

Uh oh!

BowenBao commented Mar 1, 2023 • edited by linux-foundation-easycla bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ilyasher commented Mar 1, 2023

Uh oh!

ilyasher commented Mar 1, 2023

Uh oh!

BowenBao commented Mar 2, 2023

Uh oh!

pytorchmergebot commented Mar 2, 2023

Uh oh!

pytorchmergebot commented Mar 2, 2023

Uh oh!

ilyasher commented Mar 2, 2023

Uh oh!

ilyasher commented Mar 2, 2023

Uh oh!

ilyasher commented Mar 8, 2023

Uh oh!

ilyasher commented Mar 8, 2023

Uh oh!

pytorch-bot bot commented Mar 8, 2023

Uh oh!

ilyasher commented Mar 8, 2023

Uh oh!

BowenBao commented Mar 8, 2023

Uh oh!

pytorchmergebot commented Mar 8, 2023

Uh oh!

pytorchmergebot commented Mar 8, 2023

Uh oh!

BowenBao commented Mar 8, 2023

Uh oh!

pytorchmergebot commented Mar 9, 2023

Merge started

Uh oh!

pytorchmergebot commented Mar 9, 2023

Merge failed

Uh oh!

BowenBao commented Mar 10, 2023

Uh oh!

pytorchmergebot commented Mar 10, 2023

Merge started

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pytorch-bot bot commented Feb 27, 2023 •

edited

Loading

linux-foundation-easycla bot commented Feb 27, 2023 •

edited

Loading

BowenBao commented Mar 1, 2023 •

edited by linux-foundation-easycla bot

Loading