Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20, extend opset 18 support #118828

liqunfu · 2024-02-01T01:00:28Z

Start to fix #114801

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

pytorch-bot · 2024-02-01T01:00:32Z

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit 15c31d0 with merge base 37e5632 ():

NEW FAILURE - The following job has failed:

pull / linux-jammy-py3.8-gcc11 / build (gh)
Could not assume role with user credentials: User: arn:aws:sts::308535385114:assumed-role/gh-ci-github-action-runners-runner-role/i-060fd8478151edf83 is not authorized to perform: sts:TagSession on resource: arn:aws:iam::391835788720:role/gha-pytorch-ci-artifacts-role

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

torch/onnx/_constants.py

… tests of opset > 17 Signed-off-by: liqunfu <liqun.fu@microsoft.com>

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

test/onnx/test_fx_op_consistency.py

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

torch/onnx/symbolic_opset20.py

docs/source/onnx.rst

.ci/docker/common/install_onnx.sh

test/onnx/test_pytorch_onnx_onnxruntime.py

torch/onnx/symbolic_opset20.py

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

Co-authored-by: Thiago Crepaldi <thiagofc@microsoft.com>

Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi

Overall, it looks good. Thank you for that!

My biggest concern is the if opset check within the symbolic_opsetXXX.py files. Each files belong to a specific opset already. If the same op has different implementation for different opsets, their implementation must be "duplicated" in different symbolic_opsetXXX.py files.

Maybe the tests https://github.com/pytorch/pytorch/blob/main/test/onnx/test_utility_funs.py#L276 and https://github.com/pytorch/pytorch/blob/main/test/onnx/test_utility_funs.py#L260 need to be removed?

torch/onnx/symbolic_opset20.py

torch/onnx/symbolic_helper.py

torch/onnx/symbolic_opset9.py

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi · 2024-03-14T14:51:55Z

Suppressing bc-linter check because overload_by_arg_count is only being moved to another file - not being removed. It is not supposed to be public too

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi

The PR is nearly there :)

thiagocrepaldi · 2024-03-18T17:13:08Z

torch/onnx/symbolic_opset9.py

+    else:
+        mean = g.op(
+            "ReduceMean",
+            input,
+            g.op("Constant", value_t=torch.tensor(axes, dtype=torch.long)),
+        )
+


should we move this to symbolic_opset18.py or create a helper at symbolic_helper.py?

Implementation on this method is not moved to helper because it uses opset9 add, sub, pow, sqrt. It would either introduce circular dependency or above methods need to be refactored/moved as well. For this reason, it is better to leave this method here.

Normally I would go with refactoring add, sub, pow and sqrt to keep things consistent, but TorchScript is being deprecated, so maybe it is ok to keep as-is

torch/onnx/symbolic_helper.py

thiagocrepaldi · 2024-03-18T17:14:27Z

torch/onnx/symbolic_opset9.py

+    else:
+        variance = g.op(
+            "ReduceMean",
+            pow(g, numerator, two_cst),
+            g.op("Constant", value_t=torch.tensor(axes, dtype=torch.long)),
+        )


should we move this to symbolic_opset18.py or create a helper at symbolic_helper.py?

same as above comment for the reason this method stays here.

Addressed

thiagocrepaldi · 2024-03-22T17:59:33Z

@pytorchbot merge -f "unrelated flake CI test"

pytorchmergebot · 2024-03-22T18:01:20Z

Merge started

Your change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Please use -f as last resort and instead consider -i/--ignore-current to continue the merge ignoring current failures. This will allow currently pending tests to finish and report signal before the merge.

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

thiagocrepaldi · 2024-03-22T18:11:49Z

Excellent work, thank you!

…0, extend opset 18 support (#118828) Start to fix #114801 Co-authored-by: Thiago Crepaldi <thiagofc@microsoft.com> Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com> Pull Request resolved: #118828 Approved by: https://github.com/thiagocrepaldi

Update default opset for the torchscript exporter to 18 to match the dynamo exporter, because support was actaully added and tested in #118828. In the next version we should plan to update to opset 21 or higher. This change also removes the hard limit on the torchscript exporter for more flexibility. Pull Request resolved: #156023 Approved by: https://github.com/Skylion007

symbolic_opset20.py with GridSample and AffineGrid

e2d9888

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

liqunfu requested review from BowenBao, abock, thiagocrepaldi and wschin as code owners February 1, 2024 01:00

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

pytorchbot added the open source label Feb 1, 2024

thiagocrepaldi added module: onnx Related to torch.onnx onnx-triaged triaged by ONNX team labels Feb 1, 2024

liqunfu marked this pull request as draft February 1, 2024 16:48

BowenBao reviewed Feb 1, 2024

View reviewed changes

torch/onnx/_constants.py Outdated Show resolved Hide resolved

liqunfu added 5 commits February 2, 2024 01:08

adding opset and ort tests for affinegrid and gridsample, skip reduce…

f25d2b5

… tests of opset > 17 Signed-off-by: liqunfu <liqun.fu@microsoft.com>

skip tests known to fail due to new opset versions

2ecae9e

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

update ort and onnx version

c089ee2

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

minor changes

ff3e395

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

Merge branch 'main' into liqun/onnx-opset20

8c2f906

BowenBao reviewed Feb 3, 2024

View reviewed changes

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

doc

0275556

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi reviewed Feb 5, 2024

View reviewed changes

torch/onnx/symbolic_opset20.py Show resolved Hide resolved

docs/source/onnx.rst Show resolved Hide resolved

.ci/docker/common/install_onnx.sh Outdated Show resolved Hide resolved

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

justinchuby reviewed Feb 8, 2024

View reviewed changes

torch/onnx/symbolic_opset20.py Outdated Show resolved Hide resolved

justinchuby reviewed Feb 8, 2024

View reviewed changes

torch/onnx/symbolic_opset20.py Outdated Show resolved Hide resolved

justinchuby reviewed Feb 8, 2024

View reviewed changes

torch/onnx/symbolic_opset20.py Outdated Show resolved Hide resolved

liqunfu and others added 4 commits February 8, 2024 20:57

merge main

d22c000

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

lint

b58dfbf

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

Update torch/onnx/symbolic_opset20.py

8f4ab78

Co-authored-by: Thiago Crepaldi <thiagofc@microsoft.com>

Update torch/onnx/symbolic_opset20.py

6ff1c89

Co-authored-by: Justin Chu <justinchuby@users.noreply.github.com>

liqunfu mentioned this pull request Feb 9, 2024

Tracking: add implementations for ONNX ops in opset 19 and 20 to symbolic_opset19.py and Symbolic_opset20.py #119534

Open

liqunfu added 2 commits February 9, 2024 07:55

update with reviewers' comments

b4e00c9

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

fix lint and doc ci

03b652a

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi requested a review from albanD March 12, 2024 14:54

thiagocrepaldi requested changes Mar 12, 2024

View reviewed changes

torch/onnx/symbolic_opset20.py Outdated Show resolved Hide resolved

torch/onnx/symbolic_helper.py Show resolved Hide resolved

torch/onnx/symbolic_opset9.py Outdated Show resolved Hide resolved

refactor1

2e5acd4

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

thiagocrepaldi added the suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) label Mar 14, 2024

liqunfu added 4 commits March 15, 2024 00:47

refactor2

2e0a4aa

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

refactor3

6d99bf9

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

refactor 4

9eb3d00

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

refactor 5

1d8b561

Signed-off-by: liqunfu <liqun.fu@microsoft.com>

liqunfu changed the title ~~Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20 (Partially)~~ Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20 (Partially), extend opset 18 support Mar 15, 2024

liqunfu changed the title ~~Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20 (Partially), extend opset 18 support~~ Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20, extend opset 18 support Mar 15, 2024

Merge branch 'main' into liqun/onnx-opset20

15c31d0

thiagocrepaldi reviewed Mar 18, 2024

View reviewed changes

thiagocrepaldi approved these changes Mar 22, 2024

View reviewed changes

pytorchmergebot added the merging label Mar 22, 2024

pytorchmergebot added the Merged label Mar 22, 2024

pytorchmergebot closed this in bbe846f Mar 22, 2024

pytorchmergebot removed the merging label Mar 22, 2024

thiagocrepaldi mentioned this pull request Apr 1, 2024

Exporter for ONNX GroupNormalization operator #90878

Closed

This was referenced Aug 28, 2024

[feature request]: Update max onnx opset to 21 for onnxruntime==1.18 compatability #127167

Closed

[ONNX] Allow opset 21 for ONNXRuntime==1.18 compatibility (#127167) #134571

Closed

This was referenced Jun 15, 2025

Add flags to allow dynamo usage & provide export pathway for onnx opset >= 21 huggingface/optimum#2282

Closed

[ONNX] Update default opset to 18 #156023

Closed

Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20, extend opset 18 support #118828

Add symbolic_opset19.py and symbolic_opset20.py to support opset 19/20, extend opset 18 support #118828

Uh oh!

Conversation

liqunfu commented Feb 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Feb 1, 2024 • 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/118828

❌ 1 New Failure

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thiagocrepaldi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

thiagocrepaldi commented Mar 14, 2024

Uh oh!

thiagocrepaldi left a comment

Choose a reason for hiding this comment

Uh oh!

thiagocrepaldi Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

liqunfu Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

thiagocrepaldi Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thiagocrepaldi Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

liqunfu Mar 18, 2024

Choose a reason for hiding this comment

Uh oh!

thiagocrepaldi commented Mar 22, 2024

Uh oh!

pytorchmergebot commented Mar 22, 2024

Merge started

Uh oh!

thiagocrepaldi commented Mar 22, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

liqunfu commented Feb 1, 2024 •

edited

Loading

pytorch-bot bot commented Feb 1, 2024 •

edited

Loading