-
Notifications
You must be signed in to change notification settings - Fork 26.3k
[reland][dynamo][guards] Consider tensors as immutable for dict tag matches #141085
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/141085
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit ed46a22 with merge base f47aac6 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
|
@anijain2305 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
318e1fc to
114fab0
Compare
|
@anijain2305 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge |
Merge startedYour 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 |
|
For some reason, this causes large regression for an internal model. |
|
@pytorchbot revert -m "internal regression" -c nosignal |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
@anijain2305 your PR has been successfully reverted. |
…ct tag matches (#141085)" This reverts commit 8bfc009. Reverted #141085 on behalf of https://github.com/williamwen42 due to internal regression ([comment](#141085 (comment)))
Merge startedYour 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 |
|
@pytorchbot revert -m 'The diff D66211131 has been commandeered internally and is it not part of the train anymore. If codev is needed, pls reland this accordingly' -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
@anijain2305 your PR has been successfully reverted. |
…ct tag matches (#141085)" This reverts commit 1bf9830. Reverted #141085 on behalf of https://github.com/huydhn due to The diff D66211131 has been commandeered internally and is it not part of the train anymore. If codev is needed, pls reland this accordingly ([comment](#141085 (comment)))
|
@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
The added test fails when inline_inbuilt_nn_modules is False. |
|
@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge |
Merge startedYour 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 |
Merge failedReason: 1 jobs have failed, first few of them are: Meta Internal-Only Changes Check Details for Dev Infra teamRaised by workflow job |
|
@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge |
Merge startedYour 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 |
Reland - #139560
As mentioned in #130341, using
static py::objectcan lead to segfaults. I suspect this is the reason for the import system error seen internally (https://www.internalfb.com/sevmanager/view/469592). In this PR, I am removing thestaticpart. This is fine and also the right thing to do because this will catch if user changes the flag in the same process for compiling two different functions.Unfortunately, there is no easy way to trigger this segfault, so I can't write a test.
cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @chenyang78 @kadeng @chauhang @amjames