[jit] fix aten::grad to return optional list #29577

wanchaol · 2019-11-11T19:59:28Z

Stack from ghstack:

[jit] fix aten::grad to return optional list #29577 [jit] fix aten::grad to return optional list

Summary:

torch.autograd.grad can return none is one of the input is not in the
autograd graph or not requires_grad, this fix it so that it return a
list of optional tensor instead of list of tensor.

This might have BC issue unfortunately, but I think it's rare both
internal and external (only training use it, and most of the training
use backward, instead of autograd.grad), so whitelist it.

Test Plan:

Reviewers:

Subscribers:

Tasks:

Tags:

Differential Revision: D18491642

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This should not have any BC issue, because List[Tensor] :< List[Optional[Tensor]] Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This should not have any BC issue, because List[Tensor] :< List[Optional[Tensor]] Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 3ec806f Pull Request resolved: #29577

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This should not have any BC issue, because List[Tensor] :< List[Optional[Tensor]] Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This should not have any BC issue, because List[Tensor] :< List[Optional[Tensor]] Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 0bce7a6 Pull Request resolved: #29577

eellison · 2019-11-11T21:44:09Z

List[Tensor] doesn't subtype List[Optional[Tensor]]

wanchaol · 2019-11-11T23:26:54Z

List[Tensor] doesn't subtype List[Optional[Tensor]]

@eellison hmmm ok, but List[Tensor] can be passed into a function that is accepting List[Optional[Tensor]] since we try to unify the inner types, which should resolve the BC concern.

eellison · 2019-11-11T23:34:35Z

List[Tensor] doesn't subtype List[Optional[Tensor]]

@eellison hmmm ok, but List[Tensor] can be passed into a function that is accepting List[Optional[Tensor]] since we try to unify the inner types, which should resolve the BC concern.

I don't think List[Tensor] can't be passed into a function that accepts List[Optional[Tensor]]

wanchaol · 2019-11-11T23:53:40Z

List[Tensor] doesn't subtype List[Optional[Tensor]]

@eellison hmmm ok, but List[Tensor] can be passed into a function that is accepting List[Optional[Tensor]] since we try to unify the inner types, which should resolve the BC concern.

I don't think List[Tensor] can't be passed into a function that accepts List[Optional[Tensor]]

it can I think, the following code works:

@torch.jit.script 
def return_tensor_list(): 
	# type: () -> List[Tensor] 
    return [torch.randn(3, 2), torch.randn(3, 2)] 

@torch.jit.script 
def test_use_opt_list(t_list): 
    # type: (List[Optional[Tensor]]) -> Optional[Tensor] 
    return t_list[0]

test_use_opt_list(return_tensor_list()) 

Out[5]: 
tensor([[ 0.5223, -0.9674],
        [ 0.2795, -0.4572],
        [-0.0400, -1.8270]])

eellison · 2019-11-11T23:58:54Z

List[Tensor] doesn't subtype List[Optional[Tensor]]

@eellison hmmm ok, but List[Tensor] can be passed into a function that is accepting List[Optional[Tensor]] since we try to unify the inner types, which should resolve the BC concern.

I don't think List[Tensor] can't be passed into a function that accepts List[Optional[Tensor]]

it can I think, the following code works:
@torch.jit.script 
def return_tensor_list(): 
	# type: () -> List[Tensor] 
    return [torch.randn(3, 2), torch.randn(3, 2)] 

@torch.jit.script 
def test_use_opt_list(t_list): 
    # type: (List[Optional[Tensor]]) -> Optional[Tensor] 
    return t_list[0]

test_use_opt_list(return_tensor_list()) 

Out[5]: 
tensor([[ 0.5223, -0.9674],
        [ 0.2795, -0.4572],
        [-0.0400, -1.8270]])

That's at the pybind layer so it's different because inputs from python don't have a well-formed type yet. Try:

@torch.jit.script 
def return_tensor_list(): 
	# type: () -> List[Tensor] 
    return [torch.randn(3, 2), torch.randn(3, 2)] 

@torch.jit.script 
def test_use_opt_list(t_list): 
    # type: (List[Optional[Tensor]]) -> Optional[Tensor] 
    return t_list[0]

@torch.jit.script
def will_fail():
    test_use_opt_list(return_tensor_list())

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 19f6a87 Pull Request resolved: #29577

zdevito

We should land this, but we need to be careful because it does break BC. Why is it ok to break BC?

It is a bugfix, so the old behavior was not correct when gradients were None.
It is unlikely to be used in any TorchScript inference models.
grad was introduced relatively recently, so we do not expect a ton of serialized use of it.

Before landing, we should check that we are not breaking known predictor models.

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: b1bd342 Pull Request resolved: #29577

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18491642](https://our.internmc.facebook.com/intern/diff/D18491642) [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: 4f1c9e2 Pull Request resolved: #29577

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: [D18491642](https://our.internmc.facebook.com/intern/diff/D18491642) [ghstack-poisoned]

Summary: `torch.autograd.grad` can return none is one of the input is not in the autograd graph or not requires_grad, this fix it so that it return a list of optional tensor instead of list of tensor. This might have BC issue unfortunately, but I think it's rare both internal and external (only training use it, and most of the training use backward, instead of autograd.grad), so whitelist it. Test Plan: Reviewers: Subscribers: Tasks: Tags: ghstack-source-id: d46686b Pull Request resolved: #29577

wanchaol requested a review from apaszke as a code owner November 11, 2019 19:59

facebook-github-bot added the oncall: jit Add this issue/PR to JIT oncall triage queue label Nov 11, 2019

wanchaol requested review from suo and zdevito November 11, 2019 21:34

zdevito approved these changes Nov 13, 2019

View reviewed changes

wanchaol mentioned this pull request Nov 21, 2019

[JIT] Fix bug of not supporting retain_graph in autograd #30203

Closed

facebook-github-bot closed this in f7b12a9 Nov 21, 2019

facebook-github-bot deleted the gh/wanchaol/65/head branch December 10, 2019 15:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[jit] fix aten::grad to return optional list #29577

[jit] fix aten::grad to return optional list #29577

Uh oh!

wanchaol commented Nov 11, 2019 •

edited

Loading

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

wanchaol commented Nov 11, 2019

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

wanchaol commented Nov 11, 2019

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

zdevito left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[jit] fix aten::grad to return optional list #29577

[jit] fix aten::grad to return optional list #29577

Uh oh!

Conversation

wanchaol commented Nov 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

wanchaol commented Nov 11, 2019

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

wanchaol commented Nov 11, 2019

Uh oh!

eellison commented Nov 11, 2019

Uh oh!

zdevito left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

wanchaol commented Nov 11, 2019 •

edited

Loading

zdevito left a comment •

edited

Loading