KEMBAR78
[export] Lift constant tensors as buffers by angelayi · Pull Request #108592 · pytorch/pytorch · GitHub
Skip to content

Conversation

@angelayi
Copy link
Contributor

@angelayi angelayi commented Sep 5, 2023

When we retrace the graph containing constant tensors, they get lifted as buffer inputs.
AotInductor also wants to lift all the constants as inputs.
If we separate the constants as a separate thing, then it adds an additional complexity where we now have to keep track of 3 inputs (params, buffers, constants).

Cons: People might care about specifically what buffers are/are not buffers?

If people want to know specifically which buffers are constants, we can add an additional field in the graph signature to mark this.

Stack from ghstack (oldest at bottom):

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 5, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 30bf6d7 with merge base 96d7407 (image):
💚 Looks good so far! There are no failures yet. 💚

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

@angelayi angelayi changed the title [export] Lift constant tensors [export] Lift constant tensors as buffers Sep 5, 2023
angelayi added a commit that referenced this pull request Sep 5, 2023
ghstack-source-id: 8b4778c
Pull Request resolved: #108592
angelayi added a commit that referenced this pull request Sep 5, 2023
ghstack-source-id: 3567246
Pull Request resolved: #108592
@angelayi
Copy link
Contributor Author

angelayi commented Sep 6, 2023

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

@angelayi
Copy link
Contributor Author

angelayi commented Sep 6, 2023

@pytorchbot merge

@angelayi angelayi requested a review from suo September 6, 2023 17:56
@angelayi
Copy link
Contributor Author

angelayi commented Sep 6, 2023

@pytorchbot merge

@angelayi angelayi requested a review from gmagogsfm September 6, 2023 21:28
@angelayi
Copy link
Contributor Author

angelayi commented Sep 6, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 6, 2023
@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

@angelayi
Copy link
Contributor Author

angelayi commented Sep 6, 2023

@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

@huydhn
Copy link
Contributor

huydhn commented Sep 8, 2023

@pytorchbot revert -m 'Check with Angela, we can revert this as the internal failure on D49017872 is legit' -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

Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland.

@huydhn
Copy link
Contributor

huydhn commented Sep 8, 2023

Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland.

This looks weird as the internal diff D49017872 hasn't been landed yet. I will try to unlink it, and revert this one one more time.

@huydhn
Copy link
Contributor

huydhn commented Sep 8, 2023

@pytorchbot revert -m 'Check with Angela, we can revert this as the internal failure on D49017872 is legit' -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

Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland.

@huydhn
Copy link
Contributor

huydhn commented Sep 8, 2023

@pytorchbot revert -m 'Remove the reference to D49017872 in the summary, this should work, right?' -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

Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland.

huydhn added a commit to huydhn/pytorch that referenced this pull request Sep 8, 2023
@huydhn
Copy link
Contributor

huydhn commented Sep 8, 2023

@pytorchbot revert -m 'Remove the reference to D49017872 in the summary, this should work, right?' -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

Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland.

pytorchmergebot pushed a commit that referenced this pull request Sep 8, 2023
This reverts commit e340723.

I gave up trying to revert the original PR in the usual way #108592 (comment), so let's manually revert it then.

Pull Request resolved: #108893
Approved by: https://github.com/izaitsevfb, https://github.com/atalman
@facebook-github-bot facebook-github-bot deleted the gh/angelayi/36/head branch September 10, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk Trigger trunk jobs on your pull request Merged release notes: export

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants