Skip to content

Conversation

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 10, 2023

🔗 Helpful Links

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

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

✅ No Failures

As of commit 5156356 with merge base 4f2b288 (image):
💚 Looks good so far! There are no failures yet. 💚

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

… follow_imports typechecking"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
def insertable_tensor_check(self, t: torch.Tensor) -> bool:
# TODO - we could also Tensors which get replaced with arange here
return (
return bool(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need this cast?

Copy link
Collaborator

@Skylion007 Skylion007 Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like there is a typing bug with one of the logical and overloads. This cast would fix that issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tensor.all() below returns a Tensor rather than a bool. I'll move this bool cast to that line to make things clearer

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, that makes sense. Thank you for the clarification!

… follow_imports typechecking"

cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov ColinPeppler

[ghstack-poisoned]
return (
t.numel() != 0
and (t == t.flatten()[0]).all()
and bool((t == t.flatten()[0]).all())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT

Suggested change
and bool((t == t.flatten()[0]).all())
and (t == t.flatten()[0]).all().item()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, looks like the bool() would be necessary anyway as item() is typed as returning Union[bool, int, float, complex]. I guess ideally we'd have generics in our Tensor type...

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool is pretty much the same as item() so I think it's alright as it is

pytorchmergebot pushed a commit that referenced this pull request Nov 10, 2023
…ollow_imports typechecking (#113411)

SymIntType is referenced by wrapper.py, so I added its .pyi definition.
I also added SymBoolType along the way for completeness.

The `insinstance` checks in wrapper.py reference torch.Type, which seems
to cause mypy to choke. Not entirely sure why; I've just added
type-ignore comments for now.

Pull Request resolved: #113411
Approved by: https://github.com/Skylion007
ghstack dependencies: #113409, #113410
Skylion007 pushed a commit to Skylion007/pytorch that referenced this pull request Nov 14, 2023
Skylion007 pushed a commit to Skylion007/pytorch that referenced this pull request Nov 14, 2023
…ollow_imports typechecking (pytorch#113411)

SymIntType is referenced by wrapper.py, so I added its .pyi definition.
I also added SymBoolType along the way for completeness.

The `insinstance` checks in wrapper.py reference torch.Type, which seems
to cause mypy to choke. Not entirely sure why; I've just added
type-ignore comments for now.

Pull Request resolved: pytorch#113411
Approved by: https://github.com/Skylion007
ghstack dependencies: pytorch#113409, pytorch#113410
@facebook-github-bot facebook-github-bot deleted the gh/int3/77/head branch November 14, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants