[ONNX] Skip assertion nodes #126889

titaiwangms · 2024-05-22T17:32:07Z

Continue from #125930

The PR addresses the common existence of unused assertion nodes in models. (workaround #112443)

Assertion ops does not return anything in the graph, so it's not even an no-op in ONNX:

pytorch/aten/src/ATen/native/native_functions.yaml

Line 173 in d4ec18b

- func: _assert_async.msg(Tensor self, str assert_msg) -> ()

pytorch/aten/src/ATen/native/native_functions.yaml

Line 200 in d4ec18b

 - func: sym_constrain_range_for_size(Scalar size, *, int? min=None, int? max=None) -> () 

This pass can be extended when we see more and more none-return ops in FX.

pytorch-bot · 2024-05-22T17:32:11Z

🔗 Helpful Links

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

📄 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, 12 Unrelated Failures

As of commit 62c56bf with merge base bdc39ee ():

NEW FAILURE - The following job has failed:

pull / linux-jammy-py3.8-gcc11 / test (distributed, 2, 2, linux.2xlarge) (gh)
Process completed with exit code 1.

UNSTABLE - The following jobs failed but were likely due to flakiness present on trunk and has been marked as unstable:

pull / linux-focal-cuda12.1-py3.10-gcc9 / test (default, 2, 5, linux.4xlarge.nvidia.gpu) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-cuda12.1-py3.10-gcc9-experimental-split-build / test (default, 2, 5, linux.4xlarge.nvidia.gpu) (gh) (#129080)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 2, 5, linux.g5.4xlarge.nvidia.gpu) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.11-clang10 / test (crossref, 2, 2, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.11-clang10 / test (default, 2, 3, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.12-clang10 / test (default, 2, 3, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.12-clang10-experimental-split-build / test (default, 2, 3, linux.2xlarge, unstable) (gh) (#129248)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.8-clang10 / test (crossref, 2, 2, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-focal-py3.8-clang10 / test (default, 2, 3, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-jammy-py3.10-clang15-asan / test (default, 2, 6, linux.4xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
pull / linux-jammy-py3.8-gcc11 / test (default, 2, 3, linux.2xlarge) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported
trunk / linux-focal-cuda12.4-py3.10-gcc9-sm86 / test (default, 2, 5, linux.g5.4xlarge.nvidia.gpu) (gh) (related job)
test_public_bindings.py::TestPublicBindings::test_modules_can_be_imported

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

torch/onnx/_internal/fx/passes/__init__.py

titaiwangms · 2024-05-22T17:38:15Z

@pytorchbot merge

pytorchmergebot · 2024-05-22T17:40:40Z

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 · 2024-05-22T18:44:02Z

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:
#126896

Details for Dev Infra team

Raised by workflow job

titaiwangms · 2024-05-22T22:59:02Z

@pytorchbot merge

pytorchmergebot · 2024-05-22T23:00:54Z

Merge failed

Reason: Approvers from one of the following sets are needed:

superuser (pytorch/metamates)
Core Reviewers (mruberry, lezcano, Skylion007, ngimel, peterbell10)
Core Maintainers (soumith, gchanan, ezyang, dzhulgakov, malfet)

Details for Dev Infra team

Raised by workflow job

Failing merge rule: Core Maintainers

titaiwangms · 2024-05-22T23:06:14Z

@albanD @kit1980 @malfet PTAL test_public_bindings.py is touched.

titaiwangms · 2024-05-28T21:34:32Z

@pytorchbot rebase

pytorchmergebot · 2024-05-28T21:36:01Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-05-28T21:36:05Z

Successfully rebased titaiwang/skip_assertion_onnx onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout titaiwang/skip_assertion_onnx && git pull --rebase)

albanD · 2024-05-28T21:37:25Z

test/test_public_bindings.py

@@ -309,6 +309,7 @@ def test_modules_can_be_imported(self):
 "torch.onnx._internal.fx.op_validation",
 "torch.onnx._internal.fx.passes",
 "torch.onnx._internal.fx.passes._utils",
+ "torch.onnx._internal.fx.passes.assertion_removal",


Adding new entries here is highly discouraged as it means that trying to "import" this module leads to a hard crash.
You should consider making the module importable from generic configs (in this case the based cpu image in CI).

Thanks! I just found that the very same problem happened before: #117314, and what is happened is #117314 (comment). I wonder if there is a better way to do this. @xadupre @malfet

The goal of this test is to make sure that every module is importable, i.e. it does not rely on any implicit imports.
Is there a good reason why this should not be the case for this module. If it depends on some non-PyTorch package, it's totally fine to add it as dependency to requirements-ci.txt

titaiwangms · 2024-06-06T02:15:34Z

@pytorchbot rebase

pytorchmergebot · 2024-06-06T02:16:59Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-06-06T02:17:02Z

Successfully rebased titaiwang/skip_assertion_onnx onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout titaiwang/skip_assertion_onnx && git pull --rebase)

titaiwangms · 2024-06-13T20:44:20Z

@pytorchbot rebase

pytorchmergebot · 2024-06-13T20:45:47Z

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

pytorchmergebot · 2024-06-13T20:45:51Z

Successfully rebased titaiwang/skip_assertion_onnx onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout titaiwang/skip_assertion_onnx && git pull --rebase)

torch/onnx/_internal/fx/passes/assertion_removal.py

titaiwangms added module: onnx Related to torch.onnx onnx-triaged triaged by ONNX team labels May 22, 2024

titaiwangms requested review from justinchuby and shubhambhokare1 May 22, 2024 17:32

titaiwangms requested review from BowenBao, thiagocrepaldi and wschin as code owners May 22, 2024 17:32

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

titaiwangms mentioned this pull request May 22, 2024

[ONNX][dynamo] Skip assertion node export #125930

Closed

titaiwangms added the topic: new features topic category label May 22, 2024

justinchuby reviewed May 22, 2024

View reviewed changes

torch/onnx/_internal/fx/passes/__init__.py Outdated Show resolved Hide resolved

justinchuby approved these changes May 22, 2024

View reviewed changes

pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label May 22, 2024

titaiwangms self-assigned this May 22, 2024

pytorchmergebot added the merging label May 22, 2024

pytorchbot added the open source label May 22, 2024

pytorchmergebot removed the merging label May 22, 2024

titaiwangms requested a review from albanD as a code owner May 22, 2024 20:53

pytorchmergebot added the merging label May 22, 2024

pytorchmergebot removed the merging label May 22, 2024

titaiwangms added the onnx-needs-import This PR is related to ONNX, but touches files outside of merge rule patterns, and hence needs import label May 22, 2024

pytorchmergebot force-pushed the titaiwang/skip_assertion_onnx branch from 1a46c28 to eda75cc Compare May 28, 2024 21:36

albanD reviewed May 28, 2024

View reviewed changes

pytorchmergebot force-pushed the titaiwang/skip_assertion_onnx branch from eda75cc to 4f39494 Compare June 6, 2024 02:17

pytorchmergebot force-pushed the titaiwang/skip_assertion_onnx branch from 4f39494 to a8bc3d1 Compare June 13, 2024 20:45

justinchuby reviewed Jun 20, 2024

View reviewed changes

torch/onnx/_internal/fx/passes/assertion_removal.py Outdated Show resolved Hide resolved

justinchuby reviewed Jun 21, 2024

View reviewed changes

torch/onnx/_internal/fx/passes/assertion_removal.py Outdated Show resolved Hide resolved

titaiwangms added 5 commits June 21, 2024 17:07

[ONNX] Skip assertion nodes

433606a

addresses reviews

5f257ae

modify tests

879a7cf

fix non zero op CI fails

d847db1

delete beartype

62c56bf

titaiwangms force-pushed the titaiwang/skip_assertion_onnx branch from a8bc3d1 to 62c56bf Compare June 21, 2024 17:57

titaiwangms closed this Jul 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ONNX] Skip assertion nodes #126889

[ONNX] Skip assertion nodes #126889

titaiwangms commented May 22, 2024 •

edited

Loading

pytorch-bot bot commented May 22, 2024 •

edited

Loading

titaiwangms commented May 22, 2024

pytorchmergebot commented May 22, 2024

pytorchmergebot commented May 22, 2024

titaiwangms commented May 22, 2024

pytorchmergebot commented May 22, 2024

titaiwangms commented May 22, 2024 •

edited

Loading

titaiwangms commented May 28, 2024

pytorchmergebot commented May 28, 2024

pytorchmergebot commented May 28, 2024

albanD May 28, 2024

titaiwangms May 28, 2024

malfet Jun 20, 2024

titaiwangms commented Jun 6, 2024

pytorchmergebot commented Jun 6, 2024

pytorchmergebot commented Jun 6, 2024

titaiwangms commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

[ONNX] Skip assertion nodes #126889

[ONNX] Skip assertion nodes #126889

Conversation

titaiwangms commented May 22, 2024 • edited Loading

pytorch-bot bot commented May 22, 2024 • edited Loading

🔗 Helpful Links

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

❌ 1 New Failure, 12 Unrelated Failures

titaiwangms commented May 22, 2024

pytorchmergebot commented May 22, 2024

Merge started

pytorchmergebot commented May 22, 2024

Merge failed

titaiwangms commented May 22, 2024

pytorchmergebot commented May 22, 2024

Merge failed

titaiwangms commented May 22, 2024 • edited Loading

titaiwangms commented May 28, 2024

pytorchmergebot commented May 28, 2024

pytorchmergebot commented May 28, 2024

albanD May 28, 2024

Choose a reason for hiding this comment

titaiwangms May 28, 2024

Choose a reason for hiding this comment

malfet Jun 20, 2024

Choose a reason for hiding this comment

titaiwangms commented Jun 6, 2024

pytorchmergebot commented Jun 6, 2024

pytorchmergebot commented Jun 6, 2024

titaiwangms commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

pytorchmergebot commented Jun 13, 2024

titaiwangms commented May 22, 2024 •

edited

Loading

pytorch-bot bot commented May 22, 2024 •

edited

Loading

titaiwangms commented May 22, 2024 •

edited

Loading