Skip to content

[BE] Make PyObjectSlot use a global PyInterpreter#162659

Closed
PaliC wants to merge 17 commits intopytorch:mainfrom
PaliC:remove_pyinterpreter
Closed

[BE] Make PyObjectSlot use a global PyInterpreter#162659
PaliC wants to merge 17 commits intopytorch:mainfrom
PaliC:remove_pyinterpreter

Conversation

@PaliC
Copy link
Contributor

@PaliC PaliC commented Sep 10, 2025 •
edited by pytorch-bot bot
Loading

This pr gets rid of the pyobj_interpreter_ variable from PyObjectSlot and saves a word in the process

Gonna ask for review from @huydhn as there are some changes to CI.

Testing: imported internally and the failed android build seems to work now!

cc @H-Huang @awgu @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @pragupta @msaroufim @dcci @jgong5 @mingfeima @XiaobingSuper @sanchitintel @ashokei @jingxu10 @jerryzh168 @aditew01 @EikanWang @voznesenskym @penguinwu @Guobing-Chen @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben @jataylo @Lucaskabela @ezyang @chenyang78

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 10, 2025 •
edited
Loading

🔗 Helpful Links

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

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

❌ 1 New Failure, 1 Unrelated Failure

As of commit e28389b with merge base ad2f731 (image):

NEW FAILURE - The following job has failed:

FLAKY - The following job failed but was likely due to flakiness present on trunk:

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

@PaliC PaliC added the topic: not user facing topic category label Sep 12, 2025
@PaliC PaliC marked this pull request as ready for review September 12, 2025 20:23
albanD
albanD approved these changes Sep 12, 2025
View reviewed changes
Copy link
Collaborator

@albanD albanD left a comment

Choose a reason for hiding this comment

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

Ok!

@PaliC
Copy link
Contributor Author

PaliC commented Sep 12, 2025

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 12, 2025
@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 12, 2025

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

pytorchmergebot commented Sep 12, 2025

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@PaliC
Copy link
Contributor Author

PaliC commented Sep 13, 2025

@pytorchbot rebase

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 13, 2025

@pytorchbot started a rebase job onto refs/remotes/origin/viable/strict. Check the current status here

@PaliC @pytorchmergebot
[BE] Make PyObjectSlot use a global PyInterpreter
051e544 
ghstack-source-id: 826f3f0
Pull Request resolved: pytorch#158409

[BE] Remove pyinterpreter struct from pyobjslot

ghstack-source-id: 826f3f0
Pull Request resolved: pytorch#159511
@PaliC @pytorchmergebot
check
d5a496e
@PaliC @pytorchmergebot
oops
973c3b5
@PaliC @pytorchmergebot
fixed i think
7856f8d
@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 13, 2025

Successfully rebased remove_pyinterpreter onto refs/remotes/origin/viable/strict, please pull locally before adding more changes (for example, via git checkout remove_pyinterpreter && git pull --rebase)

@PaliC
Copy link
Contributor Author

PaliC commented Sep 13, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 13, 2025

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

pytorchmergebot commented Sep 13, 2025

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@PaliC PaliC requested a review from a team as a code owner September 15, 2025 21:37
@PaliC PaliC requested a review from huydhn September 15, 2025 21:40
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 25, 2025

@PaliC has imported this pull request. If you are a Meta employee, you can view this in D83214133.

@PaliC
Copy link
Contributor Author

PaliC commented Sep 25, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 25, 2025

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

pytorchmergebot commented Sep 25, 2025

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

@PaliC
Copy link
Contributor Author

PaliC commented Sep 25, 2025

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 25, 2025

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

pytorchmergebot commented Sep 25, 2025

Merge failed

Reason: 1 mandatory check(s) failed. The first few are:

Dig deeper by viewing the failures on hud

Details for Dev Infra team Raised by workflow job

Failing merge rule: Core Maintainers

@PaliC
Copy link
Contributor Author

PaliC commented Sep 25, 2025

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 25, 2025

Merge started

Your change will be merged while ignoring the following 3 checks: pull / linux-jammy-py3.13-clang12 / test (default, 3, 5, lf.linux.4xlarge), Limited CI for symmetric memory tests on H100 / linux-jammy-cuda12.8-py3.10-gcc11-sm90-symm / test (h100-symm-mem, 1, 1, linux.aws.h100.4), vllm-test / ci-vllm-test / build

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

pytorchmergebot commented Sep 25, 2025

Merge failed

Reason: 1 jobs have failed, first few of them are: trunk / win-vs2022-cpu-py3 / test (default, 1, 3, lf.windows.4xlarge.nonephemeral)

Details for Dev Infra team Raised by workflow job

@PaliC
Copy link
Contributor Author

PaliC commented Sep 25, 2025

@pytorchbot merge -i

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Sep 25, 2025

Merge started

Your change will be merged while ignoring the following 4 checks: pull / linux-jammy-py3.13-clang12 / test (default, 3, 5, lf.linux.4xlarge), trunk / win-vs2022-cpu-py3 / test (default, 1, 3, lf.windows.4xlarge.nonephemeral), Limited CI for symmetric memory tests on H100 / linux-jammy-cuda12.8-py3.10-gcc11-sm90-symm / test (h100-symm-mem, 1, 1, linux.aws.h100.4), vllm-test / ci-vllm-test / build

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

@ysiraichi ysiraichi mentioned this pull request Sep 30, 2025
Merged
@izaitsevfb
Copy link
Contributor

izaitsevfb commented Oct 5, 2025

@pytorchbot revert -m "reverted internally, see D83214133" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Oct 5, 2025

@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

pytorchmergebot commented Oct 5, 2025

Reverting PR 162659 failed

Reason: Command git -C /home/runner/work/pytorch/pytorch revert --no-edit 29cbcbac4215e0d9070a1b7a07ddaec9a36bbd08 returned non-zero exit code 1

Auto-merging .github/workflows/pull.yml
Auto-merging c10/core/impl/PyInterpreterHooks.h
Auto-merging c10/core/impl/PyObjectSlot.h
CONFLICT (content): Merge conflict in c10/core/impl/PyObjectSlot.h
Auto-merging torch/csrc/Module.cpp
Auto-merging torch/csrc/Storage.cpp
Auto-merging torch/csrc/autograd/python_variable.cpp
error: could not revert 29cbcbac421... [BE] Make PyObjectSlot use a global PyInterpreter (#162659)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git revert --continue".
hint: You can instead skip this commit with "git revert --skip".
hint: To abort and get back to the state before "git revert",
hint: run "git revert --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Details for Dev Infra team Raised by workflow job

@izaitsevfb izaitsevfb mentioned this pull request Oct 5, 2025
Closed
@izaitsevfb
Copy link
Contributor

izaitsevfb commented Oct 5, 2025

@pytorchbot revert -m "reverted internally, see D83214133" -c ghfirst

@pytorchmergebot
Copy link
Collaborator

pytorchmergebot commented Oct 5, 2025

@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

pytorchmergebot commented Oct 5, 2025

@PaliC your PR has been successfully reverted.

@ezyang ezyang mentioned this pull request Nov 6, 2025
Closed
@github-actions
Copy link
Contributor

github-actions bot commented Jan 19, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@huydhn huydhn huydhn approved these changes

@albanD albanD albanD approved these changes

@mikaylagawarecki mikaylagawarecki Awaiting requested review from mikaylagawarecki mikaylagawarecki is a code owner

@soulitzer soulitzer Awaiting requested review from soulitzer soulitzer is a code owner

@janeyx99 janeyx99 Awaiting requested review from janeyx99

@XuehaiPan XuehaiPan Awaiting requested review from XuehaiPan

@eqy eqy Awaiting requested review from eqy

@syed-ahmed syed-ahmed Awaiting requested review from syed-ahmed

@avikchaudhuri avikchaudhuri Awaiting requested review from avikchaudhuri

@tugsbayasgalan tugsbayasgalan Awaiting requested review from tugsbayasgalan

@zhxchen17 zhxchen17 Awaiting requested review from zhxchen17

@ydwu4 ydwu4 Awaiting requested review from ydwu4

@angelayi angelayi Awaiting requested review from angelayi

@divyanshk divyanshk Awaiting requested review from divyanshk

@ramanishsingh ramanishsingh Awaiting requested review from ramanishsingh

@ezyang ezyang Awaiting requested review from ezyang

@Chillee Chillee Awaiting requested review from Chillee

@sraikund16 sraikund16 Awaiting requested review from sraikund16

@kulinseth kulinseth Awaiting requested review from kulinseth

@malfet malfet Awaiting requested review from malfet

@zou3519 zou3519 Awaiting requested review from zou3519

@lezcano lezcano Awaiting requested review from lezcano

@nikitaved nikitaved Awaiting requested review from nikitaved

@IvanYashchuk IvanYashchuk Awaiting requested review from IvanYashchuk

@desertfire desertfire Awaiting requested review from desertfire

@shunting314 shunting314 Awaiting requested review from shunting314

@jeffdaily jeffdaily Awaiting requested review from jeffdaily

@jataylo jataylo Awaiting requested review from jataylo

@jithunnair-amd jithunnair-amd Awaiting requested review from jithunnair-amd

@pruthvistony pruthvistony Awaiting requested review from pruthvistony

@titaiwangms titaiwangms Awaiting requested review from titaiwangms

@justinchuby justinchuby Awaiting requested review from justinchuby

@jerryzh168 jerryzh168 Awaiting requested review from jerryzh168

@jbschlosser jbschlosser Awaiting requested review from jbschlosser

@fmassa fmassa Awaiting requested review from fmassa

Assignees

No one assigned

Labels

ci-no-td Do not run TD on this PR ciflow/h100-symm-mem ciflow/inductor ciflow/mps Run MPS tests (subset of trunk) ciflow/trunk Trigger trunk jobs on your pull request ciflow/vllm Merged module: cpu CPU specific problem (e.g., perf, algorithm) module: dynamo module: inductor NNC oncall: distributed Add this issue/PR to distributed oncall triage queue open source release notes: distributed (checkpoint) release notes: inductor (aoti) release notes: quantization release notes category Reverted Stale topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@PaliC @pytorchmergebot @jeanschmidt @wdvr @facebook-github-bot @izaitsevfb @huydhn @albanD @pytorchbot