-
-
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
Conversation
Thx! After updating commit-messages, I'll check again. |
Unfortunately torch is failing with:
|
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.
file INSTALL cannot make directory
"/nix/store/747rhv8hh2w8kqwyni7jcfqzxpldxxcv-python3-3.11.9/lib/python3.11/site-packages/caffe2":
Well I'd guess something must be passing the wrong installation prefix. Specifically something in there looks at sys.executable
in python
(or something equivalent) and piggybacks it into some of the install prefixes. Grepping the source, I see there's TORCH_INSTALL_LIB_DIR
which I guess is always relative and shouldn't result in this error. Grepping for caffe2, there's nothing interesting either:
❯ ag 'install.*/caffe2'
scripts/onnx/install.sh
36:pip install -r "$top_dir/caffe2/requirements.txt"
scripts/onnx/install-develop.sh
16:pip install -r "$top_dir/caffe2/requirements.txt"
setup.py
723: # tmp_install/lib/pythonM.m/site-packages/caffe2/python/
caffe2/CMakeLists.txt
1920: install(DIRECTORY ${CMAKE_BINARY_DIR}/caffe2 DESTINATION ${PYTHON_LIB_REL_PATH}
cmake/Codegen.cmake
44:install(DIRECTORY ${CMAKE_CURRENT_LIST_DIR}/../caffe2
52:install(FILES ${CMAKE_BINARY_DIR}/caffe2/core/macros.h
There's tools/setup_helpers/cmake.py
setting CMAKE_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 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.
The CPU build works if I replace the cmake build
invocation with:
${cmake}/bin/cmake -DPYTHON_LIB_REL_PATH=$out/${python.sitePackages} build
(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 !
|
3ab0500
to
3d9c443
Compare
The CPU version build fine, but CUDA fails with:
|
It's odd, because the changed CMake regex: is part of 2.4.0. |
Ok, the issue is that (Or the cmake derivation should be patched to modify the regexp to recognize the |
It seems wrong that |
75a3a7d
to
5a924b6
Compare
Result of 176 packages marked as broken and skipped:
333 packages failed to build:
|
Result of 68 packages marked as broken and skipped:
94 packages failed to build:
568 packages built:
|
Result of 202 packages marked as broken and skipped:
90 packages failed to build:
409 packages built:
|
|
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 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 comment
The 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 comment
The 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 comment
The reason will be displayed to describe this comment to others. Learn more.
Well, unfortunately, after 2h of compilation, it failed with:
CMake Error at cmake_install.cmake:138 (file):
file INSTALL cannot find
"/build/source/cmake/Modules/FindCUDAToolkit.cmake": No such file or
directory.
FAILED: CMakeFiles/install.util
cd /build/source/build && /nix/store/aqckch626lg0vxh41dabyzrq0jx7gdk5-cmake-3.29.6/bin/cmake -P cmake_install.cmake
ninja: build stopped: subcommand failed.
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.
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 comment
The 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 comment
The 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 (PYTORCH_INSTALL_VENDORED_CMAKE_MODULES
or something)
Short-term: yes, I suppose it's OK that you just patch out the installs if you don't want to bother with the guards... although the guard patch would be probably even smaller. Your call
- 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/ |
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.
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)
Result of 96 packages marked as broken and skipped:
80 packages failed to build:
544 packages built:
|
53a3005
to
6a750ce
Compare
Diff: pytorch/pytorch@v2.3.1...v2.4.0 Changelog: https://github.com/pytorch/pytorch/releases/tag/v2.4.0 Co-authored-by: =?UTF-8?q?Danie=CC=88l=20de=20Kok?= <[email protected]> Co-authored-by: Connor Baker <[email protected]>
Result of 68 packages marked as broken and skipped:
96 packages failed to build:
566 packages built:
Result of 96 packages marked as broken and skipped:
82 packages failed to build:
542 packages built:
Result of 202 packages marked as broken and skipped:
88 packages failed to build:
411 packages built:
Result of 224 packages marked as broken and skipped:
86 packages failed to build:
363 packages built:
|
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.
I skimmed again very briefly and this looks good to go?
I think so too. |
Let's catch the rest of the failures on unstable |
Our next goal: #328247 |
Description of changes
Update the torch ecosystem:
torch[-bin]
: 2.3.0 -> 2.3.1 https://github.com/pytorch/pytorch/releases/tag/v2.3.1cc @teh @thoughtpolice @tscholak
cc @junjihashimoto
torchaudio[-bin]
: 2.3.0 -> 2.3.1 https://github.com/pytorch/audio/releases/tag/v2.3.1cc @junjihashimoto
torchvision[-bin]
: 0.18.0 -> 0.18.1 https://github.com/pytorch/vision/releases/tag/v0.18.1cc @ericsagnes
cc @junjihashimoto
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.