Enable torch.tensor typechecks #45077

walterddr · 2020-09-21T18:06:03Z

this fixes #42983.

dr-ci · 2020-09-21T18:16:47Z

💊 CI failures summary and remediations

As of commit c78539d (more details on the Dr. CI page):

2/2 failures possibly* introduced in this PR
- 2/2 non-CircleCI failure(s)

Extra GitHub checks: 1 failed

Failed: Codecov - codecov/patch

codecov.io: 1 failed

Failed: codecov/patch

This comment was automatically generated by Dr. CI (expand for details).

Follow this link to opt-out of these comments for your Pull Requests.

Please report bugs/suggestions on the GitHub issue tracker or post in the (internal) Dr. CI Users group.

See how this bot performed.

This comment has been revised 33 times.

several ignores are put in due to limitation on mypy

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

codecov · 2020-09-22T20:24:28Z

Codecov Report

Merging #45077 into master will decrease coverage by 0.00%.
The diff coverage is 66.66%.

@@            Coverage Diff             @@
##           master   #45077      +/-   ##
==========================================
- Coverage   67.83%   67.83%   -0.01%     
==========================================
  Files         384      384              
  Lines       50050    50060      +10     
==========================================
+ Hits        33953    33958       +5     
- Misses      16097    16102       +5

Impacted Files	Coverage Δ
torch/types.py	`77.41% <60.00%> (-9.54%)`	⬇️
torch/tensor.py	`88.72% <71.42%> (+0.04%)`	⬆️
torch/testing/_internal/expecttest.py	`77.55% <0.00%> (-1.03%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a0aa69...c78539d. Read the comment docs.

ezyang · 2020-09-23T00:49:18Z

tools/pyi/gen_pyi.py

@@ -536,6 +531,7 @@ def gen_pyi(declarations_path, out):
                     'def __init__(self, other: Tensor) -> None: ...',
                     'def __init__(self, size: {}, *, {}) -> None: ...'.format(type_to_python('IntArrayRef'), DEVICE_PARAM),
                     ],
+        'as_subclass': ["def as_subclass(self, cls) -> Tensor: ..."],


Would be nice to have a more accurate type here but I won't block this PR on it.

ezyang · 2020-09-23T00:49:40Z

tools/pyi/gen_pyi.py

@@ -576,6 +573,10 @@ def gen_pyi(declarations_path, out):
               ],
        'item': ["def item(self) -> Number: ..."],
        'copy_': ["def copy_(self, src: Tensor, non_blocking: _bool=False) -> Tensor: ..."],
+        'set_': ['def set_(self, storage: Storage, offset: _int, size: Size, stride: Tuple[_int]) -> Tensor: ...',


You mean Tuple[_int, ...] here; otherwise it's a single element tuple

ezyang · 2020-09-23T00:50:13Z

tools/pyi/gen_pyi.py

@@ -576,6 +573,10 @@ def gen_pyi(declarations_path, out):
               ],
        'item': ["def item(self) -> Number: ..."],
        'copy_': ["def copy_(self, src: Tensor, non_blocking: _bool=False) -> Tensor: ..."],
+        'set_': ['def set_(self, storage: Storage, offset: _int, size: Size, stride: Tuple[_int]) -> Tensor: ...',
+                 'def set_(self, storage: Storage) -> Tensor: ...'],
+        'split': ['def split(self, split_size: _int, dim: _int=0) -> Union[Tuple[Tensor, ...], List[Tensor]]: ...',


If you don't want to specify what the return type is, just say Sequence[Tensor]. But I think we guarantee that we return a list here.

ezyang · 2020-09-23T00:51:08Z

tools/pyi/gen_pyi.py

+        'set_': ['def set_(self, storage: Storage, offset: _int, size: Size, stride: Tuple[_int]) -> Tensor: ...',
+                 'def set_(self, storage: Storage) -> Tensor: ...'],
+        'split': ['def split(self, split_size: _int, dim: _int=0) -> Union[Tuple[Tensor, ...], List[Tensor]]: ...',
+                  'def split(self, split_size: Tuple[_int], dim: _int=0) -> Union[Tuple[Tensor, ...], List[Tensor]]: ...'],


ezyang · 2020-09-23T00:52:58Z

Some minor errors but it LGTM. There's also some small extra trailing whitespace. Thanks for doing this!

walterddr

thanks for the review @ezyang. I will clean them up asap

walterddr · 2020-09-23T15:26:03Z

torch/tensor.py

@@ -528,7 +534,7 @@ def __format__(self, format_spec):
            return self.item().__format__(format_spec)
        return object.__format__(self, format_spec)

-    def __ipow__(self, other):
+    def __ipow__(self, other):  # type: ignore[misc]


this is probably the only issue I dont know what's the right way to fix:

apparently __ipow__ is not defined in the generated _C/__init__.pyi and in previous line __pow__ = _C._TensorBase.pow instead of _C._TensorBase.__pow__ (which is defined BTW). it got me a bit confused.

I mean, reading the code here, it sounds like we don't implement __ipow__ (that's why we raise NotImplemented here). So it isn't unexpected that it's not defined in _C. What's the problem?

oops. forgot to mention the problem - mypy expects identical signature between __pow__ and __ipow__ since a **= b is equivalent to a = a ** b. Yes I think we are expected to not implemented this function so I will just leave it as ignore for now.

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

ezyang · 2020-09-23T17:33:01Z

torch/tensor.py

@@ -154,7 +160,7 @@ def __setstate__(self, state):
            raise RuntimeError('__setstate__ can be only called on leaf Tensors')
        if len(state) == 4:
            # legacy serialization of Tensor
-            self.set_(*state)
+            self.set_(*state)  


nit: try to avoid unnecessary trailing space

You can configure your editor to tell you about this. On vim I use set listchars=tab:>-,trail:·

ezyang · 2020-09-23T17:38:42Z

tools/pyi/gen_pyi.py

@@ -576,6 +573,10 @@ def gen_pyi(declarations_path, out):
               ],
        'item': ["def item(self) -> Number: ..."],
        'copy_': ["def copy_(self, src: Tensor, non_blocking: _bool=False) -> Tensor: ..."],
+        'set_': ['def set_(self, storage: Storage, offset: _int, size: Size, stride: Tuple[_int, ...]) -> Tensor: ...',


Reading this again, I don't think Tuple is the right type here, because I'm pretty sure we accept lists too here. I think using the _size type for size and stride will be more correct.

_size is defined in torch.types

malfet · 2020-09-23T18:17:21Z

torch/types.py

        ...

-    def _new_shared(self, int) -> 'Storage':


Why are you deleting _new_shared() here?

I was reordering them based on alphabetical order :-)

facebook-github-bot

@walterddr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot · 2020-09-24T16:19:27Z

@walterddr merged this pull request in bea7901.

walterddr force-pushed the gh/walterddr/type_check_testing_internal_3 branch 2 times, most recently from e6fae89 to c23ca86 Compare September 22, 2020 04:37

walterddr changed the title ~~initial commit for type checking on tensor.py~~ Enable torch.tensor typechecks Sep 22, 2020

walterddr force-pushed the gh/walterddr/type_check_testing_internal_3 branch from c23ca86 to f4e8106 Compare September 22, 2020 15:26

enable type checking on tensor.py

8c82951

several ignores are put in due to limitation on mypy

walterddr force-pushed the gh/walterddr/type_check_testing_internal_3 branch from f4e8106 to 8c82951 Compare September 22, 2020 16:01

facebook-github-bot reviewed Sep 22, 2020

View reviewed changes

walterddr marked this pull request as ready for review September 22, 2020 20:50

walterddr requested review from malfet and ezyang September 22, 2020 20:50

ezyang reviewed Sep 23, 2020

View reviewed changes

walterddr commented Sep 23, 2020

View reviewed changes

address diff comments

ddd0db5

facebook-github-bot reviewed Sep 23, 2020

View reviewed changes

ezyang reviewed Sep 23, 2020

View reviewed changes

ezyang approved these changes Sep 23, 2020

View reviewed changes

malfet reviewed Sep 23, 2020

View reviewed changes

malfet approved these changes Sep 23, 2020

View reviewed changes

address diff comments

c78539d

facebook-github-bot reviewed Sep 23, 2020

View reviewed changes

facebook-github-bot closed this in bea7901 Sep 24, 2020

facebook-github-bot added the merged label Sep 24, 2020

mruberry added the Merged label Oct 28, 2020

facebook-github-bot deleted the gh/walterddr/type_check_testing_internal_3 branch January 27, 2021 18:26

Enable torch.tensor typechecks #45077

Enable torch.tensor typechecks #45077

Uh oh!

Conversation

walterddr commented Sep 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dr-ci bot commented Sep 21, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💊 CI failures summary and remediations

Extra GitHub checks: 1 failed

codecov.io: 1 failed

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ezyang commented Sep 23, 2020

Uh oh!

walterddr left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot left a comment

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Sep 24, 2020

Uh oh!

Uh oh!

walterddr commented Sep 21, 2020 •

edited

Loading

dr-ci bot commented Sep 21, 2020 •

edited

Loading

codecov bot commented Sep 22, 2020 •

edited

Loading