-
Notifications
You must be signed in to change notification settings - Fork 7k
Add compatibility checks for C++ extensions #2467
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,10 @@ | ||
_HAS_OPS = False | ||
|
||
|
||
def _has_ops(): | ||
return False | ||
|
||
|
||
def _register_extensions(): | ||
import os | ||
import importlib | ||
|
@@ -23,10 +27,26 @@ def _register_extensions(): | |
try: | ||
_register_extensions() | ||
_HAS_OPS = True | ||
|
||
def _has_ops(): # noqa: F811 | ||
return True | ||
except (ImportError, OSError): | ||
pass | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for addressing this! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the review, I forgot to answer this.
Given that torchscript doesn't support globals, I think the only way we could have this is to have pre-canned error messages that are used to construct the Something like try:
_register_extensions()
...
except (ImportError, OSError) as err:
err_str = repr(err)
if err_type_1 in err_str:
def _has_ops():
...
elif err_type_2 in err_str:
... which makes it a bit less readable, and kinds of defeats the purpose of having the error message being returned because we would only be able to nicely print a few types of errors (the ones we hard-coded). But maybe I'm missing something here? |
||
|
||
|
||
def _assert_has_ops(): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I should do something similar for the video and image ops, but I'll do that in a follow-up PR once we define more precisely if this implementation is enough |
||
if not _has_ops(): | ||
raise RuntimeError( | ||
"Couldn't load custom C++ ops. This can happen if your PyTorch and " | ||
"torchvision versions are incompatible, or if you had errors while compiling " | ||
"torchvision from source. For further information on the compatible versions, check " | ||
"https://github.com/pytorch/vision#installation for the compatibility matrix. " | ||
"Please check your PyTorch version with torch.__version__ and your torchvision " | ||
"version with torchvision.__version__ and verify if they are compatible, and if not " | ||
"please reinstall torchvision so that it matches your PyTorch install." | ||
) | ||
|
||
|
||
def _check_cuda_version(): | ||
""" | ||
Make sure that CUDA versions match between the pytorch install and torchvision install | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can probably be removed now