-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[Refactor] Build cython with isolate environment #18124
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
Conversation
…etup.py by removing unused import
|
Am I the only one getting this error after this commit? (cc @tqchen) |
|
@vacu9708 That's strange - it works fine in my workspace. Could you please run python -c "import sys; print(sys.path)" and check whether Cython is included in the path? I also noticed that Cython likely isn't listed as a requirement on the Python side. I'm wondering what would happen if you try running |
|
@LeiWang1999 I've confirmed that the build succeeds with this commit after "sudo pip install cython". |
|
@vacu9708 Thanks for your report, I think one possible solution would be to refactor the Cython build phase into command lines. This would give us a more flexible method for finding the Cython executable and building Cython modules, and it would also enable caching def get_cython_compiler() -> Optional[str]:
"""Return the path to the Cython compiler.
Returns
-------
out: Optional[str]
The path to the Cython compiler, or None if none was found.
"""
cython_names = ["cython", "cython3"]
# Check system PATH
dirs_in_path = list(os.get_exec_path())
# Add user site-packages bin directory
user_base = site.getuserbase()
if user_base:
user_bin = os.path.join(user_base, "bin")
if os.path.exists(user_bin):
dirs_in_path = [user_bin] + dirs_in_path
# If in a virtual environment, add its bin directory
if sys.prefix != sys.base_prefix:
venv_bin = os.path.join(sys.prefix, "bin")
if os.path.exists(venv_bin):
dirs_in_path = [venv_bin] + dirs_in_path
for cython_name in cython_names:
for d in dirs_in_path:
cython_path = os.path.join(d, cython_name)
if os.path.isfile(cython_path) and os.access(cython_path, os.X_OK):
return cython_path
return Noneand build .pyx files from command line. with open(cython_wrapper_path, "r") as f:
cython_wrapper_code = f.read()
cache_dir = get_cache_dir()
source_path = cache_dir / "cython_wrapper.cpp"
library_path = cache_dir / "cython_wrapper.so"
md5_path = cache_dir / "md5.txt"
code_hash = hashlib.sha256(cython_wrapper_code.encode()).hexdigest()
cache_path = cache_dir / f"{code_hash}.so"
lock_file = cache_path.with_suffix('.lock')
# Check if cached version exists and is valid
need_compile = True
if md5_path.exists() and library_path.exists():
with open(md5_path, "r") as f:
cached_hash = f.read().strip()
if cached_hash == code_hash:
logger.debug("Cython jit adapter is up to date, no need to compile...")
need_compile = False
else:
logger.info("Cython jit adapter is out of date, need to recompile...")
else:
logger.info("No cached version found for cython jit adapter, need to compile...")
if need_compile:
logger.info("Waiting for lock to compile cython jit adapter...")
with open(lock_file, 'w') as lock:
fcntl.flock(lock.fileno(), fcntl.LOCK_EX)
try:
# After acquiring the lock, check again if the file has been compiled by another process
if md5_path.exists() and library_path.exists():
with open(md5_path, "r") as f:
cached_hash = f.read().strip()
if cached_hash == code_hash:
logger.info(
"Another process has already compiled the file, using it...")
need_compile = False
if need_compile:
logger.info("Compiling cython jit adapter...")
temp_path = cache_dir / f"temp_{code_hash}.so"
with open(md5_path, "w") as f:
f.write(code_hash)
# compile the cython_wrapper.pyx file into .cpp
cython = get_cython_compiler()
if cython is None:
raise Exception("Cython is not installed, please install it first.")
os.system(f"{cython} {cython_wrapper_path} --cplus -o {source_path}")
python_include_path = sysconfig.get_path("include")
cc = get_cplus_compiler()
command = f"{cc} -shared -pthread -fPIC -fwrapv -O2 -Wall -fno-strict-aliasing -I{python_include_path} {source_path} -o {temp_path}"
os.system(command)
# rename the temp file to the library file
temp_path.rename(library_path)
except Exception as e:
if 'temp_path' in locals() and temp_path.exists():
temp_path.unlink()
raise Exception(f"Failed to compile cython jit adapter: {e}") from e
finally:
if lock_file.exists():
lock_file.unlink()
# add the .so file to the sys.path
cache_dir_str = str(cache_dir)
if cache_dir_str not in sys.path:
sys.path.append(cache_dir_str)example codes are from https://github.com/tile-ai/tilelang/blob/main/tilelang/jit/adapter/cython/adapter.py cc @Hzfengsy |
|
@vacu9708 My experience differs from yours. I tried uninstalling all Cython installations from my system, and when I run but works after I reinstalled cython via pip [lei_py39] (lei) wanglei@wanglei-3969141-0:/weka-hg/prod/deepseek/permanent/wanglei/pr_workspace/rebase_upstream/3rdparty/tvm/build$ pip install cython
Looking in indexes: http://mirrors.aliyun.com/pypi/simple/
Collecting cython
Downloading http://mirrors.aliyun.com/pypi/packages/18/90/7dc13bd8621b25caa57d6047938ac5e324c763828fccfe956c2a38b22f90/cython-3.1.2-cp39-cp39-manylinux_2_17_x86_64.manylinux2014_x86_64.whl (3.3 MB)
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 3.3/3.3 MB 20.8 MB/s eta 0:00:00
Installing collected packages: cython
Successfully installed cython-3.1.2
[lei_py39] (lei) wanglei@wanglei-3969141-0:/weka-hg/prod/deepseek/permanent/wanglei/pr_workspace/rebase_upstream/3rdparty/tvm/build$ make -j
[ 2%] Built target project_libbacktrace
[ 2%] Built target tvm_libinfo_objs
[ 3%] Built target tvm_ffi_objs
[ 12%] Built target tvm_runtime_objs
[ 13%] Built target tvm_ffi_static
[ 13%] Built target tvm_ffi_shared
[ 13%] Built target tvm_runtime
[100%] Built target tvm_objs
[100%] Built target tvm
WARNING:root:git describe: fatal: No names found, cannot describe anything.
, use 0.21.dev0
Compiling tvm/ffi/cython/core.pyx because it changed.
[1/1] Cythonizing tvm/ffi/cython/core.pyx
INFO:root:running build_ext
INFO:root:building 'tvm.ffi.core' extension
INFO:root:g++ -pthread -B /hf_shared/hfai_envs/wanglei/lei_py39_0/compiler_compat -Wno-unused-result -Wsign-compare -DNDEBUG -O2 -Wall -fPIC -O2 -isystem /hf_shared/hfai_envs/wanglei/lei_py39_0/include -I/hf_shared/hfai_envs/wanglei/lei_py39_0/include -fPIC -O2 -isystem /hf_shared/hfai_envs/wanglei/lei_py39_0/include -fPIC -I../ffi/include/ -I../ffi/3rdparty/dlpack/include -I/hf_shared/hfai_envs/wanglei/lei_py39_0/include/python3.9 -c tvm/ffi/cython/core.cpp -o build/temp.linux-x86_64-cpython-39/tvm/ffi/cython/core.o -std=c++17 -DDMLC_USE_LOGGING_LIBRARY=<tvm/runtime/logging.h>
INFO:root:g++ -pthread -B /hf_shared/hfai_envs/wanglei/lei_py39_0/compiler_compat -Wno-unused-result -Wsign-compare -DNDEBUG -O2 -Wall -fPIC -O2 -isystem /hf_shared/hfai_envs/wanglei/lei_py39_0/include -I/hf_shared/hfai_envs/wanglei/lei_py39_0/include -fPIC -O2 -isystem /hf_shared/hfai_envs/wanglei/lei_py39_0/include -pthread -B /hf_shared/hfai_envs/wanglei/lei_py39_0/compiler_compat -shared build/temp.linux-x86_64-cpython-39/tvm/ffi/cython/core.o -o build/lib.linux-x86_64-cpython-39/tvm/ffi/core.cpython-39-x86_64-linux-gnu.so
INFO:root:copying build/lib.linux-x86_64-cpython-39/tvm/ffi/core.cpython-39-x86_64-linux-gnu.so -> tvm/ffi
[100%] Built target tvm_cython |
|
And likely some issue is with this CMake: # By default add cython to all build
find_package(Python)
if(NOT DEFINED ENV{CONDA_BUILD})
message(STATUS ${CMAKE_CURRENT_BINARY_DIR})
add_custom_target(
tvm_cython ALL
${Python_EXECUTABLE} -I setup.py build_ext --inplace
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/python
)
add_dependencies(tvm_cython tvm)
message("Add Cython build into the default build step")
endif()
|
|
I see, i think the issue was caused by When running in isolation mode, the python will ignore the user local installation environments. But may not ignore the global site-package. So not too ideal here. I think a better approach would be to revert the patch so we keep |
|
reverting this for now #18127 |
Update CMakeLists.txt to include Python isolation flag and clean up setup.py by removing unused import
Revert "[Refactor] Build cython with isolate environment (apache#18124)" This reverts commit c6969d7.
|
should be resolved via #18226 |
this pull request changed the Cython build command in CMakeLists.txt to use the
-Ioption because when utilizing TVM as a cmake 3rdparty module, the main environment's Python may pollute sys.path, causing Cython to not be found.