-
-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
python311Packages.{torch,torch-bin}: 2.3.1 -> 2.4.0 #329836
Changes from all commits
1b6efd0
b956192
996253b
12055c0
922b0d5
e100b56
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. This patch shouldn't be necessary any longer? pytorch/pytorch#108931 Skylion007/pytorch@62becc5 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. EDIT: the part about conflicting installs shouldn't be necessary, which is why the patch was originally introduced 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. Indeed, it seems to build without it. 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. Well, unfortunately, after 2h of compilation, it failed with:
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. Yeah but this is the INSTALL part, not the dependency part. This fails because it tries to install the stupid vendored versions of cmake modules 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. You suggest to re-instanciate the INSTALL part of the patch ? 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. Long-term: I'd try to upstream a patch that puts this bs behind a guard ( |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,51 +1,15 @@ | ||
diff --git a/CMakeLists.txt b/CMakeLists.txt | ||
index 9194e520bb0..d05fdcfb6cb 100644 | ||
index c4cd4b2c2a..e983b21353 100644 | ||
--- a/CMakeLists.txt | ||
+++ b/CMakeLists.txt | ||
@@ -1160,10 +1160,6 @@ if(BUILD_SHARED_LIBS) | ||
${PROJECT_SOURCE_DIR}/cmake/Modules_CUDA_fix | ||
DESTINATION share/cmake/Caffe2/ | ||
COMPONENT dev) | ||
- install(FILES | ||
- ${PROJECT_SOURCE_DIR}/cmake/Modules/FindCUDAToolkit.cmake | ||
- DESTINATION share/cmake/Caffe2/ | ||
- COMPONENT dev) | ||
install(FILES | ||
${PROJECT_SOURCE_DIR}/cmake/Modules/FindCUSPARSELT.cmake | ||
DESTINATION share/cmake/Caffe2/ | ||
diff --git a/cmake/public/cuda.cmake b/cmake/public/cuda.cmake | ||
index c7595774d81..4fc43771810 100644 | ||
--- a/cmake/public/cuda.cmake | ||
+++ b/cmake/public/cuda.cmake | ||
@@ -61,9 +61,15 @@ find_package(CUDAToolkit REQUIRED) | ||
cmake_policy(POP) | ||
|
||
if(NOT CMAKE_CUDA_COMPILER_VERSION VERSION_EQUAL CUDAToolkit_VERSION) | ||
- message(FATAL_ERROR "Found two conflicting CUDA versions:\n" | ||
- "V${CMAKE_CUDA_COMPILER_VERSION} in '${CUDA_INCLUDE_DIRS}' and\n" | ||
- "V${CUDAToolkit_VERSION} in '${CUDAToolkit_INCLUDE_DIRS}'") | ||
+ if(CUDA_INCLUDE_DIRS IN_LIST CUDAToolkit_INCLUDE_DIR) | ||
+ message(STATUS "CUDA_INCLUDE_DIRS is a substring of CUDAToolkit_INCLUDE_DIR. " | ||
+ "Setting CUDA_INCLUDE_DIRS to CUDAToolkit_INCLUDE_DIR.") | ||
+ set(CUDA_INCLUDE_DIRS "${CUDAToolkit_INCLUDE_DIR}") | ||
+ else() | ||
+ message(FATAL_ERROR "Found two conflicting CUDA installs:\n" | ||
+ "V${CMAKE_CUDA_COMPILER_VERSION} in '${CUDA_INCLUDE_DIRS}' and\n" | ||
+ "V${CUDAToolkit_VERSION} in '${CUDAToolkit_INCLUDE_DIR}'") | ||
+ endif() | ||
endif() | ||
|
||
if(NOT TARGET CUDA::nvToolsExt) | ||
diff --git a/tools/setup_helpers/cmake.py b/tools/setup_helpers/cmake.py | ||
index fb19b66dfba..3f83bef32fe 100644 | ||
--- a/tools/setup_helpers/cmake.py | ||
+++ b/tools/setup_helpers/cmake.py | ||
@@ -207,6 +207,8 @@ class CMake: | ||
"BUILDING_WITH_TORCH_LIBS", | ||
"CUDA_HOST_COMPILER", | ||
"CUDA_NVCC_EXECUTABLE", | ||
+ "CUDAToolkit_ROOT", | ||
+ "CUDAToolkit_INCLUDE_DIR", | ||
"CUDA_SEPARABLE_COMPILATION", | ||
"CUDNN_LIBRARY", | ||
"CUDNN_INCLUDE_DIR", | ||
@@ -1319,10 +1319,6 @@ if(BUILD_SHARED_LIBS) | ||
DIRECTORY ${PROJECT_SOURCE_DIR}/cmake/Modules_CUDA_fix | ||
DESTINATION share/cmake/Caffe2/ | ||
COMPONENT dev) | ||
- install( | ||
- FILES ${PROJECT_SOURCE_DIR}/cmake/Modules/FindCUDAToolkit.cmake | ||
- DESTINATION share/cmake/Caffe2/ | ||
- COMPONENT dev) | ||
install( | ||
FILES ${PROJECT_SOURCE_DIR}/cmake/Modules/FindCUSPARSELT.cmake | ||
DESTINATION share/cmake/Caffe2/ | ||
Comment on lines
+9
to
+15
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. THat said, I do not think we want to be installing any of the vendored cmake files. It's strange we're not deleting findcusparselt.cmake (doesn't have to be addressed in this pr) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
diff --git a/tools/setup_helpers/cmake.py b/tools/setup_helpers/cmake.py | ||
index 5481ce46031c..d50d9d547399 100644 | ||
--- a/tools/setup_helpers/cmake.py | ||
+++ b/tools/setup_helpers/cmake.py | ||
@@ -231,6 +231,7 @@ def generate( | ||
"SELECTED_OP_LIST", | ||
"TORCH_CUDA_ARCH_LIST", | ||
"TRACING_BASED", | ||
+ "PYTHON_LIB_REL_PATH", | ||
) | ||
} | ||
) |
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.
Well I'd guess something must be passing the wrong installation prefix. Specifically something in there looks at
sys.executable
inpython
(or something equivalent) and piggybacks it into some of the install prefixes. Grepping the source, I see there'sTORCH_INSTALL_LIB_DIR
which I guess is always relative and shouldn't result in this error. Grepping for caffe2, there's nothing interesting either:There's
tools/setup_helpers/cmake.py
settingCMAKE_INSTALL_PREFIX
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.
Try
cmakeFlags = [ "--trace-expand" ]
or(lib.cmakeBool "CMAKE_VERBOSE_MAKEFILE" true)
to figure out at least which variable is being assigned the interpreter's pathThere 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.
The CPU build works if I replace the
cmake build
invocation with:(Should probably be in
cmakeFlags
.)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.
@GaetanLepage patch (with nicer fix than my previous comment):
https://gist.github.com/danieldk/c5d7ab52139ae6a863d295dbd2a19a41
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.
Thank you very much for the patch @danieldk !