fix issue with lift_fresh_copy when using export + compile #108243
Conversation
[ghstack-poisoned]
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/108243
Note: Links to docs will display an error until the docs builds have been completed. ❗ 1 Merge Blocking SEVsThere is 1 active merge blocking SEVs. Please view them below:
If you must merge, use ✅ No FailuresAs of commit 7235651 with merge base a16b0aa ( This comment was automatically generated by Dr. CI and updates every 15 minutes. |
github-actions
bot
requested review from
albanD,
antoniojkim,
miladm,
SherlockNoMad,
voznesenskym and
wconstab
ghstack-source-id: 2aa7dc6f56f74e76764a8e2a7370bab58918dc92 Pull Request resolved: #108243
Fixes #105327. The problem is that `lift_fresh_copy()`'s functionalization implementation currently assumes that the input is always not functional. This is apparently too limiting: when you have "user" code like this (which can potentially come from exporting a model and then running compile on the resulting graph): ``` tensor_constant0 = torch.tensor(2) lift_fresh = torch.ops.aten.lift_fresh_copy.default(tensor_constant0) ``` When we run this through AOTAutograd, the first call (torch.tensor(2)) will **already** be lifted into a functional tensor wrapper - so the `lift_fresh_copy` call doesn't need to do any "lifting" anymore - it just needs to do a clone. [ghstack-poisoned]
ghstack-source-id: 8b804f8428562b51d89fa83a341641964197c131 Pull Request resolved: #108243
| TORCH_INTERNAL_ASSERT(!at::functionalization::impl::isFunctionalTensor(self)); | ||
| // See Note [Exporting and compiling a graph with lift_fresh_copy] | ||
| if (at::functionalization::impl::isFunctionalTensor(self)) { | ||
| return self; |
There was a problem hiding this comment.
You CANNOT return self here as you're below autograd.
| return self; | |
| return self.view_as(self); |
There was a problem hiding this comment.
And I thought i'd finally drilled that into my head
Fixes #105327. The problem is that `lift_fresh_copy()`'s functionalization implementation currently assumes that the input is always not functional. This is apparently too limiting: when you have "user" code like this (which can potentially come from exporting a model and then running compile on the resulting graph): ``` tensor_constant0 = torch.tensor(2) lift_fresh = torch.ops.aten.lift_fresh_copy.default(tensor_constant0) ``` When we run this through AOTAutograd, the first call (torch.tensor(2)) will **already** be lifted into a functional tensor wrapper - so the `lift_fresh_copy` call doesn't need to do any "lifting" anymore - it just needs to do a clone. [ghstack-poisoned]
ghstack-source-id: 1d7dd3ee432f11ea0434f70a2031e50ec649a9ec Pull Request resolved: #108243
Fixes #105327. The problem is that `lift_fresh_copy()`'s functionalization implementation currently assumes that the input is always not functional. This is apparently too limiting: when you have "user" code like this (which can potentially come from exporting a model and then running compile on the resulting graph): ``` tensor_constant0 = torch.tensor(2) lift_fresh = torch.ops.aten.lift_fresh_copy.default(tensor_constant0) ``` When we run this through AOTAutograd, the first call (torch.tensor(2)) will **already** be lifted into a functional tensor wrapper - so the `lift_fresh_copy` call doesn't need to do any "lifting" anymore - it just needs to do a clone. [ghstack-poisoned]
|
@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 |
Fixes #105327. The problem is that
lift_fresh_copy()'s functionalization implementation currently assumes that the input is always not functional. This is apparently too limiting: when you have "user" code like this (which can potentially come from exporting a model and then running compile on the resulting graph):When we run this through AOTAutograd, the first call (torch.tensor(2)) will already be lifted into a functional tensor wrapper - so the
lift_fresh_copycall doesn't need to do any "lifting" anymore - it just needs to do a clone.Stack from ghstack (oldest at bottom):