Skip to content

Min-cut partitioner always saves tensors that are returned as-is in backward #114970

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 18 commits into from
Closed

Min-cut partitioner always saves tensors that are returned as-is in backward #114970

wants to merge 18 commits into from

Conversation

soulitzer
Copy link
Contributor

@soulitzer soulitzer commented Dec 1, 2023 •
edited
Loading

@pytorch-bot PyTorch Bot
Copy link

pytorch-bot bot commented Dec 1, 2023 •
edited
Loading

🔗 Helpful Links

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

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

✅ You can merge normally! (1 Unrelated Failure)

As of commit e468749 with merge base 45e7af5 (image):

BROKEN TRUNK - The following job 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.

This was referenced Nov 30, 2023
Closed
Closed
Closed
Closed
Closed
Closed
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
3c1a018 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
7c30083 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
f1a51ee 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
86e4caa 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
e3a621a 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
f29b0e1 
… as-is in backward"

[ghstack-poisoned]
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
95462ec 
… as-is in backward"

[ghstack-poisoned]
@soulitzer soulitzer mentioned this pull request Dec 13, 2023
Closed
@soulitzer soulitzer mentioned this pull request Dec 13, 2023
Closed
soulitzer
soulitzer commented Dec 14, 2023
View reviewed changes
torch/_functorch/partitioners.py
# x would be part of cut nodes. A case where this happens is if
# NestedTensor saves a offset tensor as part of the singleton int
# in sizes.
nx_graph.add_edge(node.name + "_out", "sink", capacity=math.inf)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternatively we could just update the cut-set manually

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you add a test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added

@jansel
Copy link
Contributor

jansel commented Dec 16, 2023

@Chillee should review this one

@jansel jansel removed their request for review December 16, 2023 00:31
@bdhirsh bdhirsh mentioned this pull request Dec 21, 2023
Closed
@soulitzer
Update on "Min-cut partitioner always saves tensors that are returned…
e468749 
… as-is in backward"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx chenyang78 aakhundov kadeng

[ghstack-poisoned]
soulitzer added a commit that referenced this pull request Feb 8, 2024
@soulitzer
Min-cut partitioner always saves tensors that are returned as-is in b…
d8a281b 
…ackward

ghstack-source-id: 6343d3c
Pull Request resolved: #114970
@soulitzer soulitzer mentioned this pull request Feb 12, 2024
Closed
Chillee
Chillee approved these changes Feb 12, 2024
View reviewed changes
Copy link
Collaborator

@Chillee Chillee left a comment

Choose a reason for hiding this comment

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

What happens today with this? An error because it can't find a valid cut?

@soulitzer
Copy link
Contributor Author

Basically yeah, when we go to extract the modules corresponding to the input/outputs, it would error out because one of outputs of the graph is not passed as in as input anywhere.

@soulitzer
Copy link
Contributor Author

@pytorchbot merge

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

Merge failed

Reason: This PR needs a release notes: label
If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

Details for Dev Infra team Raised by workflow job

@soulitzer soulitzer added the release notes: composability release notes category label Feb 12, 2024
@soulitzer
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

clee2000 pushed a commit that referenced this pull request Feb 14, 2024
@soulitzer @clee2000
Min-cut partitioner always saves tensors that are returned as-is in b…
bbb9726 
…ackward (#114970)

Pull Request resolved: #114970
Approved by: https://github.com/Chillee
pytorchmergebot pushed a commit that referenced this pull request Feb 14, 2024
@soulitzer @pytorchmergebot
Deduplicate docs between global and non-global full backward hooks (#…
340b6fa 
…119708)

Pull Request resolved: #119708
Approved by: https://github.com/albanD
ghstack dependencies: #114970
This was referenced Feb 21, 2024
Closed
Closed
Closed
Closed
Closed
Closed
@github-actions github-actions bot deleted the gh/soulitzer/259/head branch March 14, 2024 01:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Chillee Chillee Chillee approved these changes

@bdhirsh bdhirsh Awaiting requested review from bdhirsh

Assignees

No one assigned

Labels

ciflow/inductor ciflow/trunk Trigger trunk jobs on your pull request Merged module: dynamo release notes: composability release notes category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@soulitzer @jansel @pytorchmergebot @Chillee