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

Mypy crashes when I import PyTorch 1.7.1 #9941

Closed
TylerYep opened this issue Jan 22, 2021 · 4 comments · Fixed by #11632
Closed

Mypy crashes when I import PyTorch 1.7.1 #9941

TylerYep opened this issue Jan 22, 2021 · 4 comments · Fixed by #11632

Comments

@TylerYep
Copy link

TylerYep commented Jan 22, 2021

Crash Report

Mypy crashes when I run it on a file that imports PyTorch 1.7.1 within a folder.

Traceback

...
LOG:  Writing torch.distributed.distributed_c10d /Users/tyler.yep/.pyenv/versions/3.9.0/lib/python3.9/site-packages/torch/distributed/distributed_c10d.py torch/distributed/distributed_c10d.meta.json torch/distributed/distributed_c10d.data.json
LOG:  Cached module torch.distributed.distributed_c10d has same interface
LOG:  Writing torch.distributed /Users/tyler.yep/.pyenv/versions/3.9.0/lib/python3.9/site-packages/torch/distributed/__init__.py torch/distributed/__init__.meta.json torch/distributed/__init__.data.json
LOG:  Build finished in 94.339 seconds with 741 modules, and 1 errors

Traceback (most recent call last):
  File "/Users/tyler.yep/.pyenv/versions/3.9.0/bin/mypy", line 8, in <module>
    sys.exit(console_entry())
  File "/Users/tyler.yep/.pyenv/versions/3.9.0/lib/python3.9/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "mypy/main.py", line 90, in main
  File "mypy/build.py", line 180, in build
  File "mypy/build.py", line 254, in _build
  File "mypy/build.py", line 2638, in dispatch
  File "mypy/build.py", line 2964, in process_graph
  File "mypy/build.py", line 3081, in process_stale_scc
  File "mypy/build.py", line 2252, in write_cache
  File "mypy/build.py", line 1462, in write_cache
  File "mypy/nodes.py", line 313, in serialize
  File "mypy/nodes.py", line 3140, in serialize
  File "mypy/nodes.py", line 3076, in serialize
AssertionError

To Reproduce
Place a file with the following contents in a folder:

import torch

Run mypy . from outside of that folder. (same directory as the setup.cfg)

Your Environment

  • Mypy version used: 0.800
  • Mypy command-line flags: mypy .
  • Mypy configuration options from setup.cfg:
[mypy]
strict = True
ignore_missing_imports = True
  • Python version used: 3.9.0
  • Operating system and version: Mac OS X
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 22, 2021

Thanks, can repro. Doesn't need --ignore-missing-imports. (It's also noteworthy how much slower mypy is in the cases that crash)

This could be related to #8481 where rraval took the time to minimise a repro

@vikigenius
Copy link

vikigenius commented Feb 10, 2021

I can also reproduce this, but the issue goes away if i set implicit_reexport = True

This pretty strongly indicates that, this is related to #8481

@EricWiener
Copy link

Any updates on this issue?

@sorenmc
Copy link

sorenmc commented Sep 30, 2021

implicit_reexport = True

This fixed it for us.

JukkaL pushed a commit that referenced this issue Dec 3, 2021
Fixes #8481
Fixes #9941
Fixes #11025
Fixes #11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with #11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
ilevkivskyi pushed a commit that referenced this issue Dec 3, 2021
Fixes #8481
Fixes #9941
Fixes #11025
Fixes #11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with #11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this issue Jan 20, 2022
…#11632)

Fixes python#8481
Fixes python#9941
Fixes python#11025
Fixes python#11038

This is the unresolved crash that's been reported the most times, e.g., it's as easy 
to repro as `mypy --no-implicit-reexport -c 'import pytorch_lightning'` (or even 
`import torch`, with the right PyTorch version). I know of multiple popular Python 
packages that have made changes just to work around this crash. I would love to 
see this released, potentially along with python#11630.

Thanks to @rraval for making a minimal repro!

The fix ended up being a one-liner, but it took me a bit to track down :-)

Since things worked with implicit reexport, I differentially patched in explicit reexport 
logic to narrow things down. This let me observe that if I hardcoded pkg.a.B to get 
reexported, we hit this branch, which clobbers the PlaceholderNode for pkg.c.B, 
which fixes things: 

https://github.com/python/mypy/blob/f79e7afec2c863c34d7a9b41ebb732dc26128fff/mypy/semanal.py#L2028

Which is a little weird — we shouldn't have a PlaceholderNode for pkg.c.B at all. But 
with a breakpoint in that branch, it was easy to notice that with `--no-implicit-reexport` 
pkg.a.B was first created with `module_public=True` (resulting in creation of a 
PlaceholderNode in pkg.c.B) and only on a later pass acquired `module_public=False`. 
So tracking down where pkg.a.B symbol was first created with `module_public=True` 
led me to this "obvious" bug.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants