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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove need of include Python.h in vision.cpp #3965

Closed
fmassa opened this issue Jun 4, 2021 · 1 comment 路 Fixed by #8413
Closed

Remove need of include Python.h in vision.cpp #3965

fmassa opened this issue Jun 4, 2021 · 1 comment 路 Fixed by #8413

Comments

@fmassa
Copy link
Member

fmassa commented Jun 4, 2021

馃殌 Feature

Related to #2349

We currently need to #include <Python.h> in

#include <Python.h>

as well as we need to define PyInit__C for Windows
// If we are in a Windows environment, we need to define
// initialization functions for the _custom_ops extension
#ifdef _WIN32
PyMODINIT_FUNC PyInit__C(void) {
// No need to do anything.
return NULL;
}
#endif

This is ideally not needed. Windows builds currently require it due to an artifact on how we build our C++ extensions I believe (with setuptools).

We should try to remove this. This might involve using CMake to compiling the extensions (instead of setuptools), so might involve significant rework.

Further discussion can be found in #3087 (comment)

For reference, this is how TF extensions does it for loading extensions, vs our custom code in

def _register_extensions():
import os
import importlib
import torch
# load the custom_op_library and register the custom ops
lib_dir = os.path.dirname(__file__)
if os.name == 'nt':
# Register the main torchvision library location on the default DLL path
import ctypes
import sys
kernel32 = ctypes.WinDLL('kernel32.dll', use_last_error=True)
with_load_library_flags = hasattr(kernel32, 'AddDllDirectory')
prev_error_mode = kernel32.SetErrorMode(0x0001)
if with_load_library_flags:
kernel32.AddDllDirectory.restype = ctypes.c_void_p
if sys.version_info >= (3, 8):
os.add_dll_directory(lib_dir)
elif with_load_library_flags:
res = kernel32.AddDllDirectory(lib_dir)
if res is None:
err = ctypes.WinError(ctypes.get_last_error())
err.strerror += f' Error adding "{lib_dir}" to the DLL directories.'
raise err
kernel32.SetErrorMode(prev_error_mode)
loader_details = (
importlib.machinery.ExtensionFileLoader,
importlib.machinery.EXTENSION_SUFFIXES
)
extfinder = importlib.machinery.FileFinder(lib_dir, loader_details)
ext_specs = extfinder.find_spec("_C")
if ext_specs is None:
raise ImportError
torch.ops.load_library(ext_specs.origin)
(and for image and video as well).

Given that our extensions are now completely independent from Python, we just need to know the location where they live and directly call into torch.ops.load_library.

Note that we will also need to make the solution compatible with fbcode

@fmassa
Copy link
Member Author

fmassa commented Sep 30, 2021

@prabhat00155 this might be related to the task I've assigned you. At least the part of the extension loading (hopefully we will not need to change the build, but it's not 100% sure)

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.

1 participant