Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

anjali411 · 2022-12-08T12:17:39Z

Stack from ghstack (oldest at bottom):

-> Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

cc @mlazos @soumith @voznesenskym @yanboliang @penguinwu @anijain2305 @EikanWang @jgong5 @Guobing-Chen @chunyuan-w @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @desertfire

[ghstack-poisoned]

pytorch-bot · 2022-12-08T12:17:42Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR
📄 Preview C++ docs built from this PR
❓ Need help or want to give feedback on the CI? Visit the bot commands wiki or our office hours

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

❌ 5 Failures

As of commit 7acb0be:

NEW FAILURES - The following jobs have failed:

BROKEN TRUNK - The following jobs failed but were present on the merge base 81b5eff:

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

Fixes #88843 cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

ghstack-source-id: b420f1d Pull Request resolved: #90463

torch/_dynamo/variables/tensor.py

ezyang · 2022-12-08T14:11:47Z

torch/_dynamo/variables/tensor.py

+                variables.ConstantVariable(i) for i in range(self.ndim - 1, -1, -1)
+            ]
+            args = [variables.TupleVariable(args_list)]
+            result = out.call_method(tx, "permute", args, {})


Rather than manually reimplement the semantics of T/H here, as the old code did, I think a quicker and less error prone strategy is to just generate a call to torch.Tensor.H.get, which is a function call and thus can be represented in FX IR.

>>> torch.Tensor.H.__get__(torch.randn(2, 2)) tensor([[ 0.6645, -2.2927], [ 0.7483, -0.5508]])

Awesome, I didn't know about this, maybe it'll even fix the error with 0d input that T has now.

That sounds like the right thing to do. I tried:

from .builder import wrap_fx_proxy result= wrap_fx_proxy( tx, tx.output.create_proxy( "call_function", torch.Tensor.T.__get__, (self.as_proxy(),), {}, ), **options, )

but fx chokes at __module__ query on it.

can we fix FX? It needs to recognize that it has a method wrapper, and look at the module of the class of the method to format the name

fwiw, in #91840 I tested recording these just as getattr fx nodes. (does getattr have any significant difference from using a call to torch.Tensor.H.get?) The issue I was running into was that at the point of TensorVariable.var_getattr() we don't know whether this is a tensor attribute or a tensor method.

e.g. on #91840 I was running into errors with x.fill_(), which normally falls back to a GetAttrVariable when you try to access an unknown attribute, and then once you try to call it, it turns into a TensorVariable.call_method

Fixes #88843 cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

ghstack-source-id: c867ba3 Pull Request resolved: #90463

Fixes #88843 cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

ghstack-source-id: 2473c38 Pull Request resolved: #90463

…)" This reverts commit 84266ae. ghstack-source-id: 49754d5 Pull Request resolved: #91897

…T, mH (#90463)"" This reverts commit 84266ae. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

…ributes: T, H, mT, mH (#90463)"" This reverts commit 84266ae. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

…T, mH (#90463)"" This reverts commit 84266ae. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 EikanWang jgong5 Guobing-Chen chunyuan-w XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 desertfire [ghstack-poisoned]

…)" This reverts commit 84266ae. ghstack-source-id: 6a35b88 Pull Request resolved: #91897

…)" (#91897) This reverts commit 84266ae. Pull Request resolved: #91897 Approved by: https://github.com/ngimel

@ngimel

As discussed with @ngimel, this is not only not documented, but it's also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

As discussed with ngimel, this is not only not documented, but it's also an unnecessary edge case. See #90463 (comment) ghstack-source-id: ae0ae52 Pull Request resolved: #92143

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

As discussed with ngimel, this is not only not documented, but it's also an unnecessary edge case. See #90463 (comment) ghstack-source-id: 36eb8d4 Pull Request resolved: #92143

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

As discussed with ngimel, this is not only not documented, but it's also an unnecessary edge case. See #90463 (comment) ghstack-source-id: b4e85ca Pull Request resolved: #92143

@ngimel

As discussed with @ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) Pull Request resolved: #92143 Approved by: https://github.com/ngimel

@EikanWang

…with a getattr proxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This leaves room for error, in case the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the value resulting from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. * Note: although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). * Note: prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…roxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This leaves room for error, in case the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the value resulting from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. * Note: although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). * Note: prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…with a getattr proxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…roxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…with a getattr proxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…roxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…with a getattr proxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

@EikanWang

…roxy node" **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. cc mlazos soumith voznesenskym yanboliang penguinwu anijain2305 @EikanWang jgong5 @Guobing-Chen chunyuan-w @XiaobingSuper zhuhaozhe blzheng @Xia-Weiwen wenzhe-nrv jiayisunx desertfire [ghstack-poisoned]

…91840) **Background:** Before this PR, support in dynamo for tensor attributes (e.g. `x.H`, `x.T`, ...) need to be individually implemented one-by-one. This could potentially lead to errors, e.g. if the implementation in [variables/tensor.py](https://github.com/pytorch/pytorch/blob/21c7c7c72fd13f476e08b84c45cbca3ea3f41b04/torch/_dynamo/variables/tensor.py#L160) differs from the implementation from a direct call to the attribute. For attributes that were not special-cased in tensor.py, dynamo tracing would fail. This PR adds generic support for tensor attributes that return tensors without needing to specially handle them. (Notably, for x.real and x.imag, which previously weren't supported). **In this PR:** This directly creates a proxy node for a `"call_function"` node with `target=getattr`, and feeds it into wrap_fx_proxy. This will produce a TensorVariable for the attribute returned. This also removes the implementations for H, T, mH, mT which were broken (previously `torch.relu(x.T)` would fail). They now fall back to this default implementation (for which `torch.relu(x.T)` passes). **Further context**: * Ed's original suggestion in [90463](#90463 (comment)) is to use `torch.Tensor.H.__get__(x)`. I wasn't able to get this to work; fx compilation fails with `getset_descriptor does not have attribute __module__`. Basically, the `__module__` attribute which is available on most python attributes, is not available on `getset_descriptor` objects. (i.e., these are implemented in C++ as attributes on torch.Tensor, so they don't obey some assumptions made by fx) * Although both tensor attributes and methods (like `x.relu()`) both go through this, this PR should only handle attributes (e.g. see the `"getset_descriptor"` in variables/tensor.py). Methods are handled already by by GetAttrVariable. * Prior to this PR, we already returned GetAttrVariables for unsupported attrs: the parent caller would catch the NotImplementedError and fallback to returning a GetAttrVariable. But if this GetAttrVariable was ever passed into a torch.\* function (as it could quite possibly be, since most of these attrs are tensors), it would fail because its proxy node would be missing an [example_value](https://github.com/pytorch/pytorch/blob/master/torch/_dynamo/utils.py#L1017). So: before, for some tensor x, `x.real` would work fine; but `torch.relu(x.real)` would fail. **Testing**: added tests in test_misc.py for x.real, x.imag, x.T, x.real.T. Pull Request resolved: #91840 Approved by: https://github.com/ezyang

Fix dynamo handling for tensor attributes: T, H, mT, mH

e270158

[ghstack-poisoned]

github-actions bot requested review from albanD and antoniojkim December 8, 2022 12:17

github-actions bot added ciflow/inductor module: dynamo labels Dec 8, 2022

github-actions bot requested a review from bdhirsh December 8, 2022 12:17

github-actions bot added the module: inductor label Dec 8, 2022

github-actions bot requested review from Chillee, SherlockNoMad, ezyang, miladm, voznesenskym and wconstab December 8, 2022 12:17

anjali411 added the release notes: dynamo label Dec 8, 2022

anjali411 added a commit that referenced this pull request Dec 8, 2022

Fix dynamo handling for tensor attributes: T, H, mT, mH

42ce43a

ghstack-source-id: b420f1d Pull Request resolved: #90463

anjali411 requested a review from ngimel December 8, 2022 12:29

ezyang reviewed Dec 8, 2022

View reviewed changes

torch/_dynamo/variables/tensor.py Outdated Show resolved Hide resolved

ezyang reviewed Dec 8, 2022

View reviewed changes

anjali411 added a commit that referenced this pull request Dec 9, 2022

Fix dynamo handling for tensor attributes: T, H, mT, mH

032bb47

ghstack-source-id: c867ba3 Pull Request resolved: #90463

albanD removed their request for review December 28, 2022 11:16

anjali411 added a commit that referenced this pull request Jan 8, 2023

Fix dynamo handling for tensor attributes: T, H, mT, mH

1047eb6

ghstack-source-id: 2473c38 Pull Request resolved: #90463

anjali411 added a commit that referenced this pull request Jan 9, 2023

Reland "Fix dynamo handling for tensor attributes: T, H, mT, mH (#90463…

cec2bc0

…)" This reverts commit 84266ae. ghstack-source-id: 49754d5 Pull Request resolved: #91897

anjali411 added a commit that referenced this pull request Jan 10, 2023

Reland "Fix dynamo handling for tensor attributes: T, H, mT, mH (#90463…

abfb6aa

…)" This reverts commit 84266ae. ghstack-source-id: 6a35b88 Pull Request resolved: #91897

pytorchmergebot pushed a commit that referenced this pull request Jan 10, 2023

Reland "Fix dynamo handling for tensor attributes: T, H, mT, mH (#90463…

c887837

…)" (#91897) This reverts commit 84266ae. Pull Request resolved: #91897 Approved by: https://github.com/ngimel

anjali411 closed this Jan 10, 2023

davidberard98 mentioned this pull request Jan 11, 2023

[dynamo] support tensor.mT #91262

Closed

lezcano mentioned this pull request Jan 13, 2023

Deprecate .mT,.T,.mH,.H on 0D tensors #92143

Closed

lezcano added a commit that referenced this pull request Jan 13, 2023

Deprecate .mT,.T,.mH,.H on 0D tensors

7bd1ebe

As discussed with @ngimel, this is not only not documented, but it's also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

lezcano added a commit that referenced this pull request Jan 13, 2023

Update on "[Don't Merge] Deprecate .mT,.T,.mH,.H on 0D tensors"

b10c10c

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

lezcano added a commit that referenced this pull request Jan 15, 2023

Update on "[Don't Merge] Deprecate .mT,.T,.mH,.H on 0D tensors"

b169ce7

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

lezcano added a commit that referenced this pull request Jan 15, 2023

Update on "Deprecate .mT,.T,.mH,.H on 0D tensors"

ae0e89a

As discussed with ngimel, this is not only not documented, but also an unnecessary edge case. See #90463 (comment) [ghstack-poisoned]

davidberard98 mentioned this pull request Feb 1, 2023

[dynamo] Handle general tensor attributes with a getattr proxy node #91840

Closed

facebook-github-bot deleted the gh/anjali411/235/head branch June 8, 2023 15:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

Uh oh!

anjali411 commented Dec 8, 2022 •

edited

Loading

Uh oh!

pytorch-bot bot commented Dec 8, 2022 •

edited

Loading

Uh oh!

Uh oh!

ezyang Dec 8, 2022

Uh oh!

ngimel Dec 8, 2022

Uh oh!

anjali411 Dec 9, 2022 •

edited

Loading

Uh oh!

ezyang Dec 9, 2022

Uh oh!

davidberard98 Jan 9, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

Fix dynamo handling for tensor attributes: T, H, mT, mH #90463

Uh oh!

Conversation

anjali411 commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pytorch-bot bot commented Dec 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

❌ 5 Failures

Uh oh!

Uh oh!

ezyang Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

ngimel Dec 8, 2022

Choose a reason for hiding this comment

Uh oh!

anjali411 Dec 9, 2022 • 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!

ezyang Dec 9, 2022

Choose a reason for hiding this comment

Uh oh!

davidberard98 Jan 9, 2023

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

anjali411 commented Dec 8, 2022 •

edited

Loading

pytorch-bot bot commented Dec 8, 2022 •

edited

Loading

anjali411 Dec 9, 2022 •

edited

Loading