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

Fix torchvision install due to zippeg egg #1536

Merged
merged 1 commit into from
Oct 30, 2019
Merged

Conversation

fmassa
Copy link
Member

@fmassa fmassa commented Oct 30, 2019

Fixes #1489

When compiling torchvision with zip_safe=True via python setup.py install, setuptools would generate a zippeg egg file. This made it impossible for custom extensions (which have recently been added) to work, because they would need to be unzipped beforehand.

This used to work with C++ extensions which relied on pybind11 because they were proper Python extensions, which were imported via import torchvision._C, and setuptools generated special code for handling it. This is not the case anymore with custom ops, which are just libs without any Python information.

Thanks for @tangorn and @pedrofreire for raising up the issue and giving a repro.

@fmassa fmassa requested a review from soumith October 30, 2019 13:48
Copy link
Contributor

@vincentqb vincentqb left a comment

Choose a reason for hiding this comment

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

The tests that are failing don't seem related. LGTM.

@ezyang
Copy link
Contributor

ezyang commented Oct 30, 2019

This is fine to merge, but I have a meta question which is why torchvision is using custom extensions instead of having setup.py manage compilation of all C code.

@fmassa
Copy link
Member Author

fmassa commented Oct 30, 2019

@ezyang torchvision makes use of setup.py to manage the compilation of all C code.

The issue is that in some contexts, installing torchvision would yield a zippeg egg file.

Python knows how to handle this zippeg egg file for standard python extensions (when doing import torchvision._C for example), because it generates a python import code _C.py which handles the zip file.

But for torch ops (which rely on torch.ops.load_library(path)) this is not enough because we would need to unzip the egg file to get the full path for the _C.so lib that torch.ops.load_library expects.

@fmassa fmassa merged commit 6a991f8 into pytorch:master Oct 30, 2019
@fmassa fmassa deleted the not-zip-safe branch October 30, 2019 15:33
@fmassa fmassa mentioned this pull request Nov 4, 2019
8 tasks
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.

RuntimeError: No such operator torchvision::nms
3 participants