[ONNX] Remove deprecated functions #107208

wangxiyuan · 2023-08-15T02:55:15Z

The usage of some functions is deprecated. This PR drop them.

pytorch-bot · 2023-08-15T02:55:17Z

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit 6ca3724 with merge base aee5dec ():

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

linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 4, 5, linux.g5.4xlarge.nvidia.gpu, unstable) (gh)

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

thiagocrepaldi

AFAIK torch.onnx._export is a private API, not a deprecated one.

The part that concerns me the most is that we probably don't want to add more options to the already cluttered torch.onnx.export public API.

If this PR can drop the use of _export in favor of export without adding new args to export, that would be a more reasonable change

wangxiyuan · 2023-08-29T00:59:26Z

AFAIK torch.onnx._export is a private API, not a deprecated one.

The part that concerns me the most is that we probably don't want to add more options to the already cluttered torch.onnx.export public API.

If this PR can drop the use of _export in favor of export without adding new args to export, that would be a more reasonable change

Hi, Thiago, thank for your review. I change _export to export because that I saw a deprecation messaging with _export https://github.com/pytorch/pytorch/blob/main/torch/onnx/__init__.py#L135-L139 . If it's true, export should have the same input to replace _export.

justinchuby · 2023-09-05T23:02:56Z

Thanks for catching this. In tests, we should use utils._export instead. We should also remove the _export function in init.

torch/onnx/symbolic_opset9.py

justinchuby · 2023-09-12T02:55:12Z

The call to symbolic_helper._set_onnx_shape_inference(True) in File "onnx/test_pytorch_onnx_shape_inference.py", line 44, in setUp can be removed too

torch/onnx/symbolic_helper.py

test/onnx/test_pytorch_onnx_shape_inference.py

justinchuby · 2023-09-12T23:07:23Z

@pytorchbot rebase

pytorchmergebot · 2023-09-12T23:09:29Z

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

pytorchmergebot · 2023-09-12T23:09:34Z

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

justinchuby · 2023-09-14T19:06:40Z

@pytorchbot merge -i

pytorchmergebot · 2023-09-14T19:09:46Z

Merge started

Your change will be merged while ignoring the following 1 checks: pull / linux-focal-cuda12.1-py3.10-gcc9-sm86 / test (default, 4, 5, linux.g5.4xlarge.nvidia.gpu, unstable)

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

kit1980 · 2023-09-14T21:44:03Z

I briefly looked and it looks like at least for cast_pytorch_to_onnx there are some Meta-internal usages.
High chance this will be reverted.

If it gets reverted, it's probably better to split this PR into several - one PR for one functions.
Because some internal usages may take time to resolve, splitting will allow merging the rest.

I'm also going to add check for this to https://github.com/pytorch/test-infra/tree/main/tools/torchfix

thiagocrepaldi · 2023-09-14T21:57:54Z

the fix will probably be replacing torch.onnx._export by torch.onnx.utils._export

kit1980 · 2023-09-14T22:33:31Z

the fix will probably be replacing torch.onnx._export by torch.onnx.utils._export

I see cast_pytorch_to_onnx is used explicitly in the user-side code...

justinchuby · 2023-09-14T23:31:48Z

The best thing to do will be to copy the dictionary out. Could you share a list of affected variables?

kit1980 · 2023-09-15T00:00:43Z

The best thing to do will be to copy the dictionary out. Could you share a list of affected variables?

Let's see if this gets reverted first. It's possible that the code is not really used.

…move deprecated functions (pytorch#107208)" for test or build failures Summary: This diff is reverting D49346965 D49346965: [ONNX] Remove deprecated functions (pytorch#107208) by generatedunixname499836121 has been identified to be causing the following test or build failures: Tests affected: - [mobile-vision/mobile_cv/mobile_cv/arch/tests:fb_test_jit_backend - test_jit_hta (mobile_cv.arch.tests.fb.test_jit_backend.TestJitBackend)](https://www.internalfb.com/intern/test/844425028079287/) Here's the Multisect link: https://www.internalfb.com/multisect/3068610 Here are the tasks that are relevant to this breakage: We're generating a revert to back out the changes in this diff, please note the backout may land if someone accepts it. If you believe this diff has been generated in error you may Commandeer and Abandon it. Test Plan: NA Reviewed By: amirshim Differential Revision: D49351830

kit1980 · 2023-09-19T17:05:10Z

@wangxiyuan For re-landing, let's split this PR into several - one for each removed API as I suggested before.
I'll work with you on landing those.

facebook-github-bot · 2023-09-19T17:24:45Z

@pytorchbot revert -m="Diff reverted internally" -c="ghfirst"

This Pull Request has been reverted by a revert inside Meta. To re-land this change, please open another pull request, assign the same reviewers, fix the CI failures that caused the revert and make sure that the failing CI runs on the PR by applying the proper ciflow label (e.g., ciflow/trunk).)

pytorchmergebot · 2023-09-19T17:26:43Z

@pytorchbot successfully started a revert job. Check the current status here.
Questions? Feedback? Please reach out to the PyTorch DevX Team

pytorchmergebot · 2023-09-19T17:26:53Z

@wangxiyuan your PR has been successfully reverted.

This reverts commit 263ca7d. Reverted #107208 on behalf of https://github.com/facebook-github-bot due to Diff reverted internally ([comment](#107208 (comment)))

wangxiyuan · 2023-09-20T01:02:38Z

@wangxiyuan For re-landing, let's split this PR into several - one for each removed API as I suggested before. I'll work with you on landing those.

sure, feel free to work on it. Thanks.

kit1980 · 2023-09-20T01:06:01Z

@wangxiyuan For re-landing, let's split this PR into several - one for each removed API as I suggested before. I'll work with you on landing those.

sure, feel free to work on it. Thanks.

To be clear, I'm suggesting you to create the PRs and I can help with landing by updating Meta-internal stuff.
If you're still interested of course.

wangxiyuan · 2023-09-20T01:19:37Z

@wangxiyuan For re-landing, let's split this PR into several - one for each removed API as I suggested before. I'll work with you on landing those.

sure, feel free to work on it. Thanks.

To be clear, I'm suggesting you to create the PRs and I can help with landing by updating Meta-internal stuff. If you're still interested of course.

That fine, Let's remove the deprecated APIs one by one. I'll push them later

These three functions in symbolic_helper are depreacated and should be removed after pytorch 2.0. The clean up job will be separated into several patches to ensure the safety. See: #107208 Pull Request resolved: #109681 Approved by: https://github.com/thiagocrepaldi

`_export` API was depreacated and should be removed after 2.0. See: #107208 Pull Request resolved: #109763 Approved by: https://github.com/thiagocrepaldi

kit1980 · 2023-09-26T22:28:13Z

@wangxiyuan Removal of _set_opset_version, _set_operator_export_type, and _set_onnx_shape_inference looks safe to me - maybe create a PR for those and we can quickly merge.

wangxiyuan · 2023-09-27T01:47:55Z

@wangxiyuan Removal of _set_opset_version, _set_operator_export_type, and _set_onnx_shape_inference looks safe to me - maybe create a PR for those and we can quickly merge.

It has been removed, sorry that I forgot to ping you there. I'll pay attention in the future.
#109681

wangxiyuan requested review from BowenBao, abock and thiagocrepaldi as code owners August 15, 2023 02:55

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

pytorchbot added the open source label Aug 15, 2023

wangxiyuan force-pushed the onnx_use_export branch from 7e17f73 to a3f6a60 Compare August 15, 2023 06:14

wangxiyuan changed the title ~~Drop deprecated _export usage~~ [ONNX] Drop deprecated _export usage Aug 15, 2023

wangxiyuan force-pushed the onnx_use_export branch from a3f6a60 to 1131f8d Compare August 16, 2023 02:49

wangxiyuan requested a review from wschin as a code owner August 16, 2023 02:49

soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Aug 17, 2023

thiagocrepaldi reviewed Aug 28, 2023

View reviewed changes

thiagocrepaldi added the module: onnx Related to torch.onnx label Aug 28, 2023

thiagocrepaldi mentioned this pull request Aug 28, 2023

[ONNX] Remove the deprecated func #107119

Closed

justinchuby self-assigned this Sep 5, 2023

wangxiyuan force-pushed the onnx_use_export branch 2 times, most recently from 75278d2 to 8aa6250 Compare September 12, 2023 02:00

wangxiyuan changed the title ~~[ONNX] Drop deprecated _export usage~~ [ONNX] Remove deprecated functions Sep 12, 2023

justinchuby reviewed Sep 12, 2023

View reviewed changes

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

wangxiyuan force-pushed the onnx_use_export branch from 8aa6250 to 3925889 Compare September 12, 2023 03:21

justinchuby reviewed Sep 12, 2023

View reviewed changes

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

justinchuby reviewed Sep 12, 2023

View reviewed changes

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

pytorchmergebot force-pushed the onnx_use_export branch from b9ddc29 to fcb62be Compare September 12, 2023 23:09

justinchuby approved these changes Sep 12, 2023

View reviewed changes

pytorchmergebot added the merging label Sep 14, 2023

pytorchmergebot added Merged and removed merging labels Sep 14, 2023

pytorchmergebot closed this in 263ca7d Sep 14, 2023

wangxiyuan deleted the onnx_use_export branch September 18, 2023 01:14

clee2000 mentioned this pull request Sep 19, 2023

Revert D49346965: Multisect successfully blamed "D49346965: [ONNX] Remove deprecated functions (#107208)" for test or build failures #109616

Closed

pytorchmergebot added the Reverted label Sep 19, 2023

wangxiyuan mentioned this pull request Sep 20, 2023

[ONNX] Remove the depreacated function in symbolic_helper #109681

Closed

wangxiyuan mentioned this pull request Sep 21, 2023

[ONNX] Remove the deprecated function _export #109763

Closed

[ONNX] Remove deprecated functions #107208

[ONNX] Remove deprecated functions #107208

Uh oh!

Conversation

wangxiyuan commented Aug 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Aug 15, 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/107208

✅ You can merge normally! (1 Unrelated Failure)

Uh oh!

thiagocrepaldi left a comment

Choose a reason for hiding this comment

Uh oh!

wangxiyuan commented Aug 29, 2023

Uh oh!

justinchuby commented Sep 5, 2023

Uh oh!

Uh oh!

justinchuby commented Sep 12, 2023

Uh oh!

Uh oh!

Uh oh!

justinchuby commented Sep 12, 2023

Uh oh!

pytorchmergebot commented Sep 12, 2023

Uh oh!

pytorchmergebot commented Sep 12, 2023

Uh oh!

justinchuby commented Sep 14, 2023

Uh oh!

pytorchmergebot commented Sep 14, 2023

Merge started

Uh oh!

kit1980 commented Sep 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thiagocrepaldi commented Sep 14, 2023

Uh oh!

kit1980 commented Sep 14, 2023

Uh oh!

justinchuby commented Sep 14, 2023

Uh oh!

kit1980 commented Sep 15, 2023

Uh oh!

kit1980 commented Sep 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Sep 19, 2023

Uh oh!

pytorchmergebot commented Sep 19, 2023

Uh oh!

pytorchmergebot commented Sep 19, 2023

Uh oh!

wangxiyuan commented Sep 20, 2023

Uh oh!

kit1980 commented Sep 20, 2023

Uh oh!

wangxiyuan commented Sep 20, 2023

Uh oh!

kit1980 commented Sep 26, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wangxiyuan commented Sep 27, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

wangxiyuan commented Aug 15, 2023 •

edited

Loading

pytorch-bot bot commented Aug 15, 2023 •

edited

Loading

kit1980 commented Sep 14, 2023 •

edited

Loading

kit1980 commented Sep 19, 2023 •

edited

Loading

kit1980 commented Sep 26, 2023 •

edited

Loading

wangxiyuan commented Sep 27, 2023 •

edited

Loading