Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

torch.backends not properly re-exported for static type-checking #101686

Closed
lkct opened this issue May 17, 2023 · 6 comments
Closed

torch.backends not properly re-exported for static type-checking #101686

lkct opened this issue May 17, 2023 · 6 comments
Assignees
Labels
module: python frontend For issues relating to PyTorch's Python frontend module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Comments

@lkct
Copy link
Contributor

lkct commented May 17, 2023

🐛 Describe the bug

To reproduce the error message, install VS Code+python/pylance, and enable "python.analysis.typeCheckingMode": "basic".

import torch
torch.backends.cudnn.benchmark = False

The code above works in runtime but will cause an error in static checking "backends" is not a known member of module "torch" Pylance(reportGeneralTypeIssues)


Ref: microsoft/pylance-release#4374

It is because backends is not re-exported from torch and pylance(pyright) doesn't recognize it.

(Of course, manually adding import torch.backends.cudnn in user code will make it work, but I think the intended usage is to make torch.backends.cudnn available with only import torch)

Versions

master

cc @ezyang @malfet @rgommers @xuzhao9 @gramster @albanD

@drisspg drisspg added topic: build triage review triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module labels May 17, 2023
@malfet malfet added module: python frontend For issues relating to PyTorch's Python frontend and removed topic: build labels May 22, 2023
@malfet malfet self-assigned this May 22, 2023
@malfet
Copy link
Contributor

malfet commented May 22, 2023

Hmm, I disagree that it has anything to do with build (or perhaps we need to better define what build means)
Also, please note, that those fixes are likely going to increase torch import time.
Grabbing for myself to take a look at it still.

@albanD albanD added module: typing Related to mypy type annotations and removed triage review labels May 22, 2023
@malfet
Copy link
Contributor

malfet commented May 22, 2023

Ok, looks like all it needs to do, is to have import torch.backends as backends instead of individual import torch.backends.cudnn etc

@lkct
Copy link
Contributor Author

lkct commented May 22, 2023

Also, please note, that those fixes are likely going to increase torch import time.

I don't think so (please correct me if I'm wrong). import torch.backends.cudnn should imply import torch.backends at runtime, so import torch actually should not be different after fixing.

Ok, looks like all it needs to do, is to have import torch.backends as backends instead of individual import torch.backends.cudnn etc

We also need something like from . import cudnn as cudnn in torch/backends/__init__.py.
import torch.backends.cudnn does not re-export anything (from the view of type checkers).

The same also applies to other import torch.X.Y statements if they should be exposed to the user.

@malfet
Copy link
Contributor

malfet commented May 22, 2023

Also, please note, that those fixes are likely going to increase torch import time.

I don't think so (please correct me if I'm wrong). import torch.backends.cudnn should imply import torch.backends at runtime, so import torch actually should not be different after fixing.

Doing that implies adding from torch.backends import cudnn as cudnn statements to torch/backends/__init__.py which might have been lazy imported later in the codebase, so this might affect import time and I wish Python has a mechanism to import stubs without actually importing the module.

But in all likelihood all torch.backends are already imported by default, so yes, it's unlikely to increase import time (at least not significantly)

@lkct
Copy link
Contributor Author

lkct commented May 22, 2023

torch/backends/__init__.py which might have been lazy imported later in the codebase

The torch/backends/__init__.py file is implicitly imported with import torch.backends.cudnn, even if you don't write import torch.backends. Therefore switching to import torch.backends as backends in torch/__init__.py plus from torch.backends import cudnn as cudnn in torch/backends/__init__.py does not add something new to be imported.

I wish Python has a mechanism to import stubs without actually importing the module.

Another option can be a __init__.pyi file which is used in only static checking but not runtime. However, if a pyi file is to be used, it must include ALL the public interfaces for type checkers to work (which can be quite a lot of work!).
But I don't think this will help much with import time as "lazy import" does not work in that way.

@malfet
Copy link
Contributor

malfet commented May 23, 2023

I have a fix, but I want to write some sort of regression test for it, because inspect.getmembers indeed reports that backends is a submodule of torch:

 % python -c "import inspect; import torch;print('backends' in sorted(x[0] for x in inspect.getmembers(torch, inspect.ismodule)))"
True
% pyright foo.py 
  /Users/nshulga/git/pytorch/pytorch/build/foo.py:3:7 - error: "backends" is not a known member of module "torch" (reportGeneralTypeIssues)

malfet added a commit that referenced this issue May 23, 2023
So that `pyright` is happy

Do a little refactor in `mps/__init__.py` to avoid cyclical dependency
on `torch.fx` by calling `mps._init()` implicitly.

Fixes #101686

[ghstack-poisoned]
malfet added a commit that referenced this issue May 24, 2023
So that `pyright` is happy

Do a little refactor in `mps/__init__.py` to avoid cyclical dependency
on `torch.fx` by calling `mps._init()` implicitly.

Fixes #101686

[ghstack-poisoned]
malfet added a commit that referenced this issue May 24, 2023
So that `pyright` is happy

Do a little refactor in `mps/__init__.py` to avoid cyclical dependency
on `torch.fx` by calling `mps._init()` implicitly.

Fixes #101686

ghstack-source-id: 5bc1df87ce3f43d5af849e5548ea68c20ae28911
Pull Request resolved: #102099
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: python frontend For issues relating to PyTorch's Python frontend module: typing Related to mypy type annotations triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module
Projects
None yet
Development

No branches or pull requests

4 participants