-
Notifications
You must be signed in to change notification settings - Fork 25.7k
[export] Lift constant tensors as buffers #108592
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.
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
Conversation
[ghstack-poisoned]
🔗 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 FailuresAs of commit 30bf6d7 with merge base 96d7407 ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
[ghstack-poisoned]
[ghstack-poisoned]
|
@angelayi has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@pytorchbot merge |
|
@pytorchbot merge |
|
@pytorchbot merge |
Merge failedReason: This PR needs a If not, please add the To add a label, you can comment to pytorchbot, for example For more information, see Details for Dev Infra teamRaised by workflow job |
|
@pytorchbot merge |
Merge startedYour 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 |
|
@pytorchbot revert -m 'Check with Angela, we can revert this as the internal failure on D49017872 is legit' -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
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. |
|
@pytorchbot revert -m 'Check with Angela, we can revert this as the internal failure on D49017872 is legit' -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland. |
|
@pytorchbot revert -m 'Remove the reference to D49017872 in the summary, this should work, right?' -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland. |
This reverts commit e340723.
|
@pytorchbot revert -m 'Remove the reference to D49017872 in the summary, this should work, right?' -c ghfirst |
|
@pytorchbot successfully started a revert job. Check the current status here. |
|
Can't revert PR that was landed via phabricator as D49017872. Please revert by going to the internal diff and clicking Unland. |
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
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):