Skip to content

Conversation

@anijain2305
Copy link
Contributor

@anijain2305 anijain2305 commented Nov 20, 2024

Reland - #139560

As mentioned in #130341, using static py::object can 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 the static part. 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

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 20, 2024

🔗 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 Failures

As of commit ed46a22 with merge base f47aac6 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@facebook-github-bot
Copy link
Contributor

@anijain2305 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@anijain2305 anijain2305 added the topic: not user facing topic category label Nov 20, 2024
@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Nov 20, 2024
@facebook-github-bot
Copy link
Contributor

@anijain2305 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@pytorch-bot pytorch-bot bot added the ci-no-td Do not run TD on this PR label Dec 5, 2024
@facebook-github-bot
Copy link
Contributor

@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@williamwen42
Copy link
Member

@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

@anijain2305
Copy link
Contributor Author

For some reason, this causes large regression for an internal model.

@williamwen42
Copy link
Member

@pytorchbot revert -m "internal regression" -c nosignal

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

@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

@huydhn
Copy link
Contributor

huydhn commented Dec 17, 2024

@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

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

pytorchmergebot added a commit that referenced this pull request Dec 17, 2024
…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)))
@facebook-github-bot
Copy link
Contributor

@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@williamwen42
Copy link
Member

The added test fails when inline_inbuilt_nn_modules is False.

@facebook-github-bot
Copy link
Contributor

@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@williamwen42
Copy link
Member

@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

@pytorchmergebot
Copy link
Collaborator

Merge failed

Reason: 1 jobs have failed, first few of them are: Meta Internal-Only Changes Check

Details for Dev Infra team Raised by workflow job

@facebook-github-bot
Copy link
Contributor

@williamwen42 has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@williamwen42
Copy link
Member

@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

@github-actions github-actions bot deleted the nov-13-triage branch January 19, 2025 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci-no-td Do not run TD on this PR ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo Reverted topic: not user facing topic category

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants