Skip to content

Define sparse tensors with Sparse*Tensor naming for better errors.#1361

Closed
ezyang wants to merge 2 commits intopytorch:masterfrom
ezyang:pr/sparse-tensor-naming
Closed

Define sparse tensors with Sparse*Tensor naming for better errors.#1361
ezyang wants to merge 2 commits intopytorch:masterfrom
ezyang:pr/sparse-tensor-naming

Conversation

@ezyang
Copy link
Contributor

@ezyang ezyang commented Apr 26, 2017

Previously, when you got a type error involving a tensor,
you got the following message:

AttributeError: type object 'FloatTensor' has no attribute 'foo'

This error would be the same whether or not you had a sparse or
regular tensor.

With this commit, by defining the sparse tensor classes with sparse
in their name, and then creating aliases, means that now we will
see 'SparseFloatTensor' if there is an error.

Signed-off-by: Edward Z. Yang ezyang@fb.com

@ezyang ezyang force-pushed the pr/sparse-tensor-naming branch from 53d6a2c to 01aaa11 Compare April 26, 2017 16:38
ezyang added 2 commits April 26, 2017 13:51
Previously, when you got a type error involving a tensor,
you got the following message:

    AttributeError: type object 'FloatTensor' has no attribute 'foo'

This error would be the same whether or not you had a sparse or
regular tensor.

With this commit, by defining the sparse tensor classes with sparse
in their name, and then creating aliases, means that now we will
see 'SparseFloatTensor' if there is an error.

Signed-off-by: Edward Z. Yang <ezyang@fb.com>
Signed-off-by: Edward Z. Yang <ezyang@fb.com>
@ezyang ezyang force-pushed the pr/sparse-tensor-naming branch from 01aaa11 to 38de41e Compare April 26, 2017 17:51
@ezyang
Copy link
Contributor Author

ezyang commented Apr 26, 2017

Sigh, there seems to be some magic involving names of classes which breaks when I rename to Sparse. Not sure how to proceed; fixing this might be more trouble than it's worth.

@apaszke
Copy link
Contributor

apaszke commented Apr 26, 2017

I don't think it's worth giving up our name conventions just to improve these messages. We have torch.FloatTensor and torch.cuda.FloatTensor, so it seems logical that it should be torch.sparse.FloatTensor too.

@apaszke apaszke closed this Apr 26, 2017
@ezyang
Copy link
Contributor Author

ezyang commented Apr 26, 2017

@apaszke It would be nice if we could get errors like AttributeError to disambiguate. It can't possibly be this difficult...

@apaszke
Copy link
Contributor

apaszke commented Apr 26, 2017

I agree it would be nice, but I don't think that's the correct way to do it.

@apaszke
Copy link
Contributor

apaszke commented Apr 26, 2017

Maybe we could just implement our own __getattr__ that always raises an error?

@ezyang
Copy link
Contributor Author

ezyang commented Apr 26, 2017

Yes, that sounds like it could work.

@colesbury
Copy link
Member

We could change the __name__ attribute on Tensor objects to be more specific. That would probably make other errors (like TypeErrors) more specific too.

For example:

>>> f = torch.cuda.FloatTensor()
>>> f.foobar
AttributeError: 'FloatTensor' object has no attribute 'foobar'
>>> torch.cuda.FloatTensor.__name__ = 'torch.cuda.FloatTensor'
>>> f.foobar
AttributeError: 'torch.cuda.FloatTensor' object has no attribute 'foobar'
>>> ~f
TypeError: bad operand type for unary ~: 'torch.cuda.FloatTensor'

@apaszke
Copy link
Contributor

apaszke commented Apr 26, 2017

Doesn't it break pickling? I don't think playing with __name__ to include the module is a good idea.

@colesbury
Copy link
Member

No, pickling doesn't use the __name__ attribute (at least on classes)

@colesbury
Copy link
Member

I take that back -- it's not used by Python 3 (for any protocol), but is used by Python 2

@ezyang ezyang deleted the pr/sparse-tensor-naming branch September 7, 2017 20:22
houseroad added a commit to houseroad/pytorch that referenced this pull request Aug 31, 2018
…c0898c

Summary:
Previous import was bae6333e149a59a3faa9c4d9c44974373dcf5256

Included changes:
- **[1b09eb1](onnx/onnx@1b09eb1)**: Fix the shape inference for concat (pytorch#1361) <Lu Fang>
- **[7b9b3ee](onnx/onnx@7b9b3ee)**: ONNX v1.3.0 release (pytorch#1359) <bddppq>

Differential Revision: D9615844

fbshipit-source-id: 65cb813a332859d95e9dd9dc7e089e2badb04039
facebook-github-bot pushed a commit that referenced this pull request Aug 31, 2018
…c0898c (#11153)

Summary:
Pull Request resolved: #11153

Previous import was bae6333e149a59a3faa9c4d9c44974373dcf5256

Included changes:
- **[1b09eb1](onnx/onnx@1b09eb1)**: Fix the shape inference for concat (#1361) <Lu Fang>
- **[7b9b3ee](onnx/onnx@7b9b3ee)**: ONNX v1.3.0 release (#1359) <bddppq>

Reviewed By: Ac2zoom

Differential Revision: D9615844

fbshipit-source-id: f1d4e2d6ef72a269d6ab3c1c347b272b5bdc4f2a
PenghuiCheng pushed a commit to PenghuiCheng/pytorch that referenced this pull request Sep 11, 2018
…c0898c (pytorch#11153)

Summary:
Pull Request resolved: pytorch#11153

Previous import was bae6333e149a59a3faa9c4d9c44974373dcf5256

Included changes:
- **[1b09eb1](onnx/onnx@1b09eb1)**: Fix the shape inference for concat (pytorch#1361) <Lu Fang>
- **[7b9b3ee](onnx/onnx@7b9b3ee)**: ONNX v1.3.0 release (pytorch#1359) <bddppq>

Reviewed By: Ac2zoom

Differential Revision: D9615844

fbshipit-source-id: f1d4e2d6ef72a269d6ab3c1c347b272b5bdc4f2a
eqy pushed a commit to eqy/pytorch that referenced this pull request Jan 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants