KEMBAR78
Remove onnx imports in dynamo by justinchuby · Pull Request #136334 · pytorch/pytorch · GitHub
Skip to content

Conversation

@justinchuby
Copy link
Collaborator

@justinchuby justinchuby commented Sep 19, 2024

Remove imports of the torch.onnx.operators module in dynamo. Since ONNX depends on dynamo, this import line causes a circular dependency. Judging from the source they are not actually needed.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @rec @xadupre @gramalingam @titaiwangms

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 19, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (13 Unrelated Failures)

As of commit d9f29d3 with merge base 68a7246 (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

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

@justinchuby justinchuby added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 19, 2024
Copy link
Contributor

@jansel jansel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like tests are failing, you might be able to handle this similar to https://github.com/pytorch/pytorch/blob/main/torch/_dynamo/trace_rules.py

@justinchuby justinchuby added the suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) label Sep 21, 2024
@justinchuby
Copy link
Collaborator Author

@jansel I fixed it by changing the functions to alias the aten ops. Please take another look. Thanks.

@justinchuby justinchuby requested a review from jansel September 22, 2024 04:22
@justinchuby
Copy link
Collaborator Author

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

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

@soulitzer soulitzer added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Sep 24, 2024
@justinchuby
Copy link
Collaborator Author

@pytorchbot merge -i

@justinchuby
Copy link
Collaborator Author

@pytorchbot merge -f "flaky tests"

@pytorchmergebot
Copy link
Collaborator

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

skotapati pushed a commit to kulinseth/pytorch that referenced this pull request Sep 24, 2024
Remove imports of the ``torch.onnx.operators`` module in dynamo. Since ONNX depends on dynamo, this import line causes a circular dependency. Judging from the source they are not actually needed.

Pull Request resolved: pytorch#136334
Approved by: https://github.com/xadupre, https://github.com/jansel, https://github.com/titaiwangms
BoyuanFeng pushed a commit to BoyuanFeng/pytorch that referenced this pull request Sep 25, 2024
Remove imports of the ``torch.onnx.operators`` module in dynamo. Since ONNX depends on dynamo, this import line causes a circular dependency. Judging from the source they are not actually needed.

Pull Request resolved: pytorch#136334
Approved by: https://github.com/xadupre, https://github.com/jansel, https://github.com/titaiwangms
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo module: inductor open source suppress-bc-linter Suppresses the failures of API backward-compatibility linter (Lint/bc_linter) topic: not user facing topic category triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants