[jit] bind autograd.backward and tensor.backward in TorchScript #23913
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
pytorchbot
added
oncall: jit
zdevito
reviewed
Aug 9, 2019
| @@ -665,6 +667,35 @@ void AliasDb::analyzeWait(Node* node) { | |||
| } | |||
| } | |||
|
|
|||
| void AliasDb::analyzeBackward(Node* node) { | |||
There was a problem hiding this comment.
This function doesn't handle a ton of aliasing cases. For instance, what if the grad tensor is stored in a list inside a class somewhere. A better way to do this would be to mark backward as CONSERVATIVE.
torch/csrc/jit/register_prim_ops.cpp
Outdated
| self.backward(gradient, keep_graph, create_graph); | ||
| return 0; | ||
| }, | ||
| aliasAnalysisSpecialCase()), |
There was a problem hiding this comment.
Just put aliasAnalysisConservative here and this will be correct.
There was a problem hiding this comment.
The alias_analaysis does not allow me to make it a conservative kind because of the assertion here. So what I did is that I made them the special_case kind and call analyze_conservative when handling these special cases, this happens before the assert so it should work properly.
There was a problem hiding this comment.
That assertion is bogus, just remove it.
torch/csrc/jit/register_prim_ops.cpp
Outdated
| @@ -49,7 +49,16 @@ namespace jit { | |||
|
|
|||
| namespace { | |||
|
|
|||
| template<class T> | |||
| // XXX: This function is to specialize IValue for tensor type in | |||
There was a problem hiding this comment.
This is a copy/paste that propagates a comment that is incorrect in this context. Let's just inline this instead:
auto gradient_ivalue = pop(stack);
auto gradient = gradient_ivalue.isNone() ? at::Tensor() : gradient_ivalue.toTensor()
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
…cript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
wanchaol
changed the title
…cript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913
wanchaol
added a commit
that referenced
this pull request
Aug 20, 2019
Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 6db038e Pull Request resolved: #23913
…cript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 20, 2019
Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: c15fc38 Pull Request resolved: #23913
… in TorchScript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 20, 2019
Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 97385c2 Pull Request resolved: #23913
zdevito
approved these changes
Aug 23, 2019
… autograd.backward and tensor.backward in TorchScript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 23, 2019
…n on "[jit] bind autograd.backward and tensor.backward in TorchScript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
…cript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 23, 2019
…ckward in TorchScript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
…cript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 23, 2019
…ckward in TorchScript" Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: Pull Request resolved: #23913 Differential Revision: [D16923272](https://our.internmc.facebook.com/intern/diff/D16923272)
wanchaol
added a commit
that referenced
this pull request
Aug 23, 2019
Summary: This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: d66873c Pull Request resolved: #23913
zdevito
pushed a commit
to zdevito/ATen
that referenced
this pull request
Aug 26, 2019
Summary: Pull Request resolved: pytorch/pytorch#23913 This PR bind torch.autograd.backward and tensor.backward to TorchScript, and make aliasing to the conservative for these two ops, this is mainly because backward op might write to every input tensors in the graph Test Plan: Imported from OSS Differential Revision: D16923272 fbshipit-source-id: 8a4016c62e00d00e0dee3d8c599d3aca220202f7
|
This pull request has been merged in 969c918. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Stack from ghstack:
Summary:
This PR bind torch.autograd.backward and tensor.backward to TorchScript,
and make aliasing to the conservative for these two ops, this is mainly
because backward op might write to every input tensors in the graph
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Pull Request resolved: #23913
Differential Revision: D16923272