Skip to content
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

[dynamo] Lazy disable_dynamo API out-of-dynamo #104317

Closed
wants to merge 8 commits into from

Conversation

@pytorch-bot
Copy link

pytorch-bot bot commented Jun 28, 2023

🔗 Helpful Links

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

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

✅ 1 Unrelated Failure

As of commit fc379ac:

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

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

@pytorch-bot pytorch-bot bot added the release notes: fx release notes category label Jun 28, 2023
anijain2305 added a commit that referenced this pull request Jun 28, 2023
ghstack-source-id: 51f91b27958bc9082dd49efde78b527f7c486b27
Pull Request resolved: #104317
@anijain2305 anijain2305 changed the title [rfc][dynamo] Lazy disable_dynamo API [wip][rfc][dynamo] Lazy disable_dynamo API Jun 28, 2023
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Jun 28, 2023
ghstack-source-id: ef9aaf9fcd935c31ed330a6777b3ce0049c6a81e
Pull Request resolved: #104317
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Jun 28, 2023
ghstack-source-id: 6a5a8a32c9eee89b8f441d0f9d0d6e859a10ba4c
Pull Request resolved: #104317
@albanD albanD removed their request for review June 28, 2023 14:30
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
anijain2305 added a commit that referenced this pull request Jun 28, 2023
ghstack-source-id: 5db6a23e6fce3b33f194b3cae17d79767d54ef2a
Pull Request resolved: #104317
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
Copy link
Contributor

@wconstab wconstab left a comment

Choose a reason for hiding this comment

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

wasn't someone asking if we could import dynamo from torch? would that solution make this simpler or not matter? i am ok with landing- we could continue to improve the import situation later regardless, and this would be good to unblock

@anijain2305
Copy link
Contributor Author

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Jun 29, 2023
cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
@anijain2305
Copy link
Contributor Author

wasn't someone asking if we could import dynamo from torch? would that solution make this simpler or not matter? i am ok with landing- we could continue to improve the import situation later regardless, and this would be good to unblock

Importing dynamo from torch would not help with the problem we are trying to solve (atleast in context of this #104368). Even after that PR is merged, we would have circular imports issue while trying to disable functions inside core. This PR is trying to solve that problem.

@mlazos
Copy link
Contributor

mlazos commented Jun 29, 2023

Thank you!! <3

@anijain2305
Copy link
Contributor Author

@pytorchhbot merge

@anijain2305
Copy link
Contributor Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

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

@facebook-github-bot facebook-github-bot deleted the gh/anijain2305/75/head branch July 2, 2023 14:16
huydhn pushed a commit to huydhn/pytorch that referenced this pull request Jul 4, 2023
… build failures

Summary:
This diff is reverting D47143445
D47143445: [dynamo] Lazy disable_dynamo API out-of-dynamo  (pytorch#104317) by anijain2305 has been identified to be causing the following test or build failures:

Tests affected:
- [caffe2/caffe2/fb/predictor/trt_bc:tensorrt_backwards_compatibility_test - TensorRTBackwardsCompatibilityTest.InlineCvrFp32](https://www.internalfb.com/intern/test/844424999929568/)

Here's the Multisect link:
https://www.internalfb.com/multisect/2375498
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: check D47216591

Reviewed By: 842974287

Differential Revision: D47166892

fbshipit-source-id: 37cb56b45f0ff30c87d7308360e0ded7a2d6e8fb
huydhn pushed a commit to huydhn/pytorch that referenced this pull request Jul 4, 2023
… build failures (pytorch#104607)

Summary:
Pull Request resolved: pytorch#104607

This diff is reverting D47143445
D47143445: [dynamo] Lazy disable_dynamo API out-of-dynamo  (pytorch#104317) by anijain2305 has been identified to be causing the following test or build failures:

Tests affected:
- [caffe2/caffe2/fb/predictor/trt_bc:tensorrt_backwards_compatibility_test - TensorRTBackwardsCompatibilityTest.InlineCvrFp32](https://www.internalfb.com/intern/test/844424999929568/)

Here's the Multisect link:
https://www.internalfb.com/multisect/2375498
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: check D47216591

Reviewed By: 842974287

Differential Revision: D47166892

fbshipit-source-id: 2ab5f72723e94af6b79dc160c370dfab1e60cb19
huydhn pushed a commit to huydhn/pytorch that referenced this pull request Jul 4, 2023
… build failures

Summary:
This diff is reverting D47143445
D47143445: [dynamo] Lazy disable_dynamo API out-of-dynamo  (pytorch#104317) by anijain2305 has been identified to be causing the following test or build failures:

Tests affected:
- [caffe2/caffe2/fb/predictor/trt_bc:tensorrt_backwards_compatibility_test - TensorRTBackwardsCompatibilityTest.InlineCvrFp32](https://www.internalfb.com/intern/test/844424999929568/)

Here's the Multisect link:
https://www.internalfb.com/multisect/2375498
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

Differential Revision: D47166892

fbshipit-source-id: db0d4431d7ff259274c09fe4a5a5c4cf49684b55
huydhn pushed a commit to huydhn/pytorch that referenced this pull request Jul 4, 2023
… build failures (pytorch#104607)

Summary:
Pull Request resolved: pytorch#104607

This diff is reverting D47143445
D47143445: [dynamo] Lazy disable_dynamo API out-of-dynamo  (pytorch#104317) by anijain2305 has been identified to be causing the following test or build failures:

Tests affected:
- [caffe2/caffe2/fb/predictor/trt_bc:tensorrt_backwards_compatibility_test - TensorRTBackwardsCompatibilityTest.InlineCvrFp32](https://www.internalfb.com/intern/test/844424999929568/)

Here's the Multisect link:
https://www.internalfb.com/multisect/2375498
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: check D47216591

Reviewed By: 842974287

Differential Revision: D47166892

fbshipit-source-id: 3c6e0a6fed06accc49206830f960ca1785b661ec
@huydhn
Copy link
Contributor

huydhn commented Jul 5, 2023

@pytorchbot revert -m 'This has been reverted internally by D47166892, so I need to also revert it on OSS to keep them in sync' -c ghfirst

@pytorchmergebot
Copy link
Collaborator

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

@pytorchmergebot
Copy link
Collaborator

@anijain2305 your PR has been successfully reverted.

github-actions bot pushed a commit to smalltalkman/pytorch that referenced this pull request Jul 5, 2023
This reverts commit 5c12a81.

Reverted pytorch#104317 on behalf of https://github.com/huydhn due to This has been reverted internally by D47166892, so I need to also revert it on OSS to keep them in sync ([comment](pytorch#104317 (comment)))
anijain2305 added a commit that referenced this pull request Jul 5, 2023
…ynamo"



Internal failed because of torch.deploy issues with disable_dynamo in fx/* and _jit/* files. Removing disable_dynamo for both. Added a comment in the code.

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx ipiszy chenyang78

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Jul 6, 2023
)

Internal failed because of torch.deploy issues with disable_dynamo in fx/* and _jit/* files. Removing disable_dynamo for both. Added a comment in the code.

Pull Request resolved: #104664
Approved by: https://github.com/wconstab
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 release notes: fx release notes category Reverted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants