10000 [ONNX] Migrate onnx ops decomp functions by justinchuby · Pull Request #147469 · pytorch/pytorch · GitHub
[go: up one dir, main page]

Skip to content

[ONNX] Migrate onnx ops decomp functions #147469

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: gh/justinchuby/111/base
Choose a base branch
from

Conversation

justinchuby
Copy link
Collaborator
@justinchuby justinchuby commented Feb 19, 2025

Stack from ghstack (oldest at bottom):

This is the main PR that adds the onnx decomp functions from onnxscript to pytorch to decouple torch.onnx from implementations in onnxscript. Details of this migration, including how the logic is tested are described in #139301 (comment).

Guide for reviewers

The PR include three parts:

  • Addition of the decomp logic. These are self contained onnxscript implementations under _torchlib/ops. They are individually tested by test/onnx/torchlib/test_ops.py using the added metadata in test/onnx/torchlib/ops_test_data.py and test/onnx/torchlib/extra_opinfo.py.
    • Match and replacement was done on the onnxscript torchlib code to replace "aten::xxx" keys to aten.xxx torch op overload objects for accurate registration in core.
  • Removal of redundant bridging logic: torch/onnx/_internal/exporter/_registration.py and torch/onnx/_internal/exporter/_ir_passes.py has logic to handle the decomp when they were out of core. These logic were removed.
  • Removal of a test: test/onnx/exporter/test_small_models_e2e.py has a single test for torchvision, which we do not migrate into core. So we remove it until we have a better plan to support torchvision. It is deemed acceptible because torchvision supoport was only a demo in the new exporter previously, and there are way to register torchvision functions using the torch.onnx.export(..., dynamo=True, custom_translation_table=...) api for users.

Test runtime

Added tests finish within 40 seconds on a 10-core local machine.

Previous work done

Necessary refactoring are done in #147396 and scafolding of tests and logic are added in #147392.

Migration source

Synced with ONNX Script at microsoft/onnxscript@6f9533e

Issue fixed

Fix #139301

Signed-off-by: Justin Chu justinchuby@users.noreply.github.com

[ghstack-poisoned]
@pytorch-bot pytorch-bot bot added the release notes: onnx torch.onnx related changes that should show up in the release notes label Feb 19, 2025
Copy link
pytorch-bot bot commented Feb 19, 2025

🔗 Helpful Links

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

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

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

❌ 1 New Failure, 1 Cancelled Job

As of commit 52f3cfa with merge base bd370c1 (image):

NEW FAILURE - The following job has failed:

CANCELLED JOB - The following job was cancelled. Please retry:

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

justinchuby added a commit that referenced this pull request Feb 19, 2025
Synced with ONNX Script at microsoft/onnxscript@6f9533e

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

ghstack-source-id: d34741d
Pull Request resolved: #147469
@justinchuby justinchuby added module: onnx Related to torch.onnx topic: new features topic category labels Feb 19, 2025
[ghstack-poisoned]
justinchuby added a commit that referenced this pull request Feb 19, 2025
Synced with ONNX Script at microsoft/onnxscript@6f9533e

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

ghstack-source-id: 081229b
Pull Request resolved: #147469

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby requested review from malfet and albanD February 19, 2025 19:18
[ghstack-poisoned]
justinchuby added a commit that referenced this pull request Feb 19, 2025
Synced with ONNX Script at microsoft/onnxscript@6f9533e

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

ghstack-source-id: 877bca7
Pull Request resolved: #147469

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby
Copy link
Collaborator Author

@albanD @malfet, I created this stack which includes a stack that does the necessary refactoring and scaffolding to set up for the onnx decomp logic migration. This PR is still big (18k loc) but the added logic are purely decomp logic code that we moved from onnxscript. Please let me know if this looks good to you

@justinchuby justinchuby added this to the 2.7.0 milestone Feb 19, 2025
justinchuby added a commit to justinchuby/pytorch that referenced this pull request Feb 19, 2025
Synced with ONNX Script at microsoft/onnxscript@6f9533e

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

ghstack-source-id: 877bca7
Pull Request resolved: pytorch#147469

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
[ghstack-poisoned]
@justinchuby justinchuby requested a review from xadupre February 20, 2025 00:24
justinchuby added a commit to justinchuby/pytorch that referenced this pull request Feb 20, 2025
Synced with ONNX Script at microsoft/onnxscript@6f9533e

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>

ghstack-source-id: 1cda915
Pull Request resolved: pytorch#147469

Signed-off-by: Justin Chu <justinchuby@users.noreply.github.com>
@justinchuby justinchuby modified the milestones: 2.7.0, 2.8.0 Mar 13, 2025
Copy link
Contributor

Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as Stale.
Feel free to remove the Stale label if you feel this was a mistake.
If you are unable to remove the Stale label please contact a maintainer in order to do so.
If you want the bot to never mark this PR stale again, add the no-stale label.
Stale pull requests will automatically be closed after 30 days of inactivity.

@github-actions github-actions bot added the Stale label May 12, 2025
@justinchuby justinchuby removed the Stale label May 12, 2025
@justinchuby justinchuby self-assigned this May 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: onnx Related to torch.onnx open source release notes: onnx torch.onnx related changes that should show up in the release notes topic: new features topic category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0