Skip to content

Conversation

@aorenste
Copy link
Contributor

@aorenste aorenste commented Nov 27, 2024

Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.

Stack from ghstack (oldest at bottom):

cc @H-Huang @awgu @kwen2501 @wanchaol @fegin @fduwjj @wz337 @wconstab @d4l3k @c-p-i-o @ezyang @SherlockNoMad @EikanWang @jgong5 @wenzhe-nrv @voznesenskym @penguinwu @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang @aakhundov

Differential Revision: D68921318

[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Nov 27, 2024

🔗 Helpful Links

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

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

✅ You can merge normally! (4 Unrelated Failures)

As of commit 2efc30a with merge base 1fdb4d6 (image):

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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

cc ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv voznesenskym penguinwu Guobing-Chen XiaobingSuper zhuhaozhe blzheng jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Nov 27, 2024
ghstack-source-id: 8d912a8
Pull Request resolved: #141659
@ezyang
Copy link
Contributor

ezyang commented Nov 28, 2024

This is pretty long. What's the best way to approach the review?

Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Dec 2, 2024
ghstack-source-id: 55b45ae
Pull Request resolved: #141659
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Dec 3, 2024
ghstack-source-id: 8cf33e3
Pull Request resolved: #141659
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Dec 4, 2024
ghstack-source-id: d2e3b45
Pull Request resolved: #141659
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
@aorenste aorenste marked this pull request as ready for review December 4, 2024 20:40
@aorenste
Copy link
Contributor Author

aorenste commented Dec 4, 2024

This is pretty long. What's the best way to approach the review?

I added some docs which should hopefully make it more approachable.

@aorenste aorenste requested review from jamesjwu and oulgen December 13, 2024 03:55
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
nWEIdia pushed a commit to nWEIdia/pytorch that referenced this pull request Jan 27, 2025
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.

Pull Request resolved: pytorch#141659
Approved by: https://github.com/jamesjwu
nWEIdia pushed a commit to nWEIdia/pytorch that referenced this pull request Jan 27, 2025
This reverts commit c6ad083.

Reverted pytorch#141659 on behalf of https://github.com/ZainRizvi due to Sorry but this is breaking internally, please take a look at D68694181 for more details. ([comment](pytorch#141659 (comment)))
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 27, 2025 23:09 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 27, 2025 23:09 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 27, 2025 23:09 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 27, 2025 23:09 Inactive
@pytorch-bot pytorch-bot bot temporarily deployed to upload-benchmark-results January 27, 2025 23:09 Inactive
aorenste added a commit that referenced this pull request Jan 28, 2025
Original commit changeset: 2de53b3

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Jan 28, 2025
Original commit changeset: 2de53b3

ghstack-source-id: e261ab2
Pull Request resolved: #145824
@aorenste
Copy link
Contributor Author

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

1 similar comment
@aorenste
Copy link
Contributor Author

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

aorenste added a commit that referenced this pull request Jan 30, 2025
Original commit changeset: 2de53b3

ghstack-source-id: 231e2f8
Pull Request resolved: #145824
aorenste added a commit that referenced this pull request Jan 30, 2025
…141659)"""

Original commit changeset: 2de53b3

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv voznesenskym penguinwu Guobing-Chen XiaobingSuper zhuhaozhe blzheng jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
aorenste added a commit that referenced this pull request Jan 30, 2025
Original commit changeset: 2de53b3

cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv voznesenskym penguinwu Guobing-Chen XiaobingSuper zhuhaozhe blzheng jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

[ghstack-poisoned]
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be able to serialize it - add some helpers to enable that.







cc H-Huang awgu kwen2501 wanchaol fegin fduwjj wz337 wconstab d4l3k c-p-i-o ezyang SherlockNoMad EikanWang jgong5 wenzhe-nrv voznesenskym penguinwu Guobing-Chen XiaobingSuper zhuhaozhe blzheng jiayisunx ipiszy yf225 chenyang78 kadeng muchulee8 ColinPeppler amjames desertfire chauhang aakhundov

Differential Revision: [D68785860](https://our.internmc.facebook.com/intern/diff/D68785860)

[ghstack-poisoned]
@aorenste
Copy link
Contributor Author

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

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

This was referenced Jan 31, 2025
desai0007 pushed a commit to desai0007/test-repo-pytorch that referenced this pull request Feb 26, 2025
Pickling GraphModule needs some special handling for wrapping things that normally can't be pickled - but async compile needs to pass them across a wire so we need to be$

Pull Request resolved: pytorch/pytorch#141659
Approved by: https://github.com/jamesjwu
ghstack-source-id: c02c3f2
pytorchmergebot pushed a commit that referenced this pull request May 26, 2025
this was added in #141659, the current change keep the same intention
"i do not want to fail here if i cant tell if the size is zero or not"
i am not familiar enough in the code to know if we need here a runtime check, but looking at current
impl it seems that guard_or_false is appropriate to match current behaviour  and have the same effect of guard_size_oblivious here.
Pull Request resolved: #154234
Approved by: https://github.com/bobrenjc93
ghstack dependencies: #154154, #154164, #154167, #154172
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 fx Merged module: dynamo module: inductor oncall: distributed Add this issue/PR to distributed oncall triage queue release notes: fx release notes category Reverted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants