-
-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
Make openai-triton/pytorch free again (and various fixes) #263048
Conversation
Draft until I make sure |
Need to change torch to propagate /nix/store/iyqd8a1w2cg54xpafi8cygjavlgrnip3-python3.11-torch-2.0.1/lib/python3.11/site-packages/torch/include/torch/csrc/Exceptions.h:14:10: fatal error: pybind11/pybind11.h: No such file or directory
> 14 | #include <pybind11/pybind11.h> As well as other minor fixes to packages that depend on torch/triton. |
Result of 36 packages marked as broken and skipped:
107 packages failed to build:
405 packages built:
|
Had to manually abort |
Is this for |
pkgs/development/python-modules/openai-triton/0000-strip-and-replace-ptxas.patch
Outdated
Show resolved
Hide resolved
|
Result of 60 packages marked as broken and skipped:
36 packages failed to build:
493 packages built:
|
Manually killed python-qutip because I didn't have the patience to let the tests finish and I need to do yet another mass rebuild anyway. |
8971c98
to
31aff0d
Compare
This is mainly for fixing fairseq's python 3.11 build
The repo was moved from PyTorchLightning/metrics to Lightning-AI/torchmetrics
Does this PR break darwin support? |
H'm? Upstream does mention macos somewhere: https://github.com/openai/triton/blob/f168b148ecdd067205c6066bc3e6939fd67ab893/python/setup.py#L77-L81. We never tested the darwin version in nixpkgs before, because it used to depend on |
Oh. Then someone has to add darwin support to nixpkgs/pkgs/by-name/op/openai-triton-llvm/package.nix Lines 120 to 123 in 2e6aa8f
|
It's using the same LLVM as before just as a separate package, and before that was using rocm-llvm which definitely did not have Darwin support. |
This is an artifact from |
Wait, actually it's not. It's been a while since I messed with openai-triton-llvm... |
So, how about having a PR that make this nixpkgs/pkgs/development/python-modules/torch/default.nix Lines 358 to 361 in f533a76
conditional again for platforms where nixpkgs/pkgs/development/python-modules/torch/default.nix Lines 361 to 368 in 47f07ca
|
I think it would be best to add a conditional. |
I think I understand why that guard is there now. |
I'd normally like to test it on ofborg, but the aarch64-linux builder is down, unfortunately... |
Here's a patch that I haven't tested at all. |
Pytorch and openai-triton is definitely on the list for aarch64-linux, because #256324 is specifically about supporting jetsons |
For the time being it makes sense to make triton optional for other platforms. I also imagine that even in future there may be users that only need the eager execution part of pytorch, and they would benefit from excluding |
From what I remember when I tried to use the latest LLVM/Triton, ROCm doesn’t support ignore the work on an updater. |
Does this break torch on x86_64-darwin/aarch64-darwin/aarch64-linux? https://hydra.nixos.org/job/nixpkgs/trunk/python311Packages.torch.x86_64-darwin |
It shouldn't break |
|
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.
Please fix these issues 🙏.
|
||
disabled = pythonOlder "3.8"; | ||
pyproject = true; | ||
disabled = pythonOlder "3.7"; |
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.
Just out of curiosity, why is this needed? When we don't recurseIntoAttrs python37.pkgs
since a long time now.
@@ -18,8 +19,7 @@ buildPythonPackage rec { | |||
pname = "mayavi"; | |||
version = "4.8.1"; | |||
format = "setuptools"; | |||
|
|||
disabled = pythonOlder "3.8"; | |||
disabled = pythonOlder "3.8" || pythonAtLeast "3.11"; |
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 PR, nor the commit message, nor the nix file itself includes a comment explaining why is this disabled. Note also f409f15 which perhaps was merged in parallel without you knowing. I also wonder how is this relevant to the PR :)
@@ -6660,8 +6662,7 @@ self: super: with self; { | |||
maya = callPackage ../development/python-modules/maya { }; | |||
|
|||
mayavi = pkgs.libsForQt5.callPackage ../development/python-modules/mayavi { | |||
inherit buildPythonPackage pythonOlder; | |||
inherit (self) pyface pygments numpy packaging vtk traitsui envisage apptools pyqt5; | |||
inherit (self) buildPythonPackage pythonOlder pythonAtLeast pyface pygments numpy packaging vtk traitsui envisage apptools pyqt5; |
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.
Also don't forget to revert the change for mayavi from here.
Reverting mayavi changes in #310111 . The author of this PR should have taken responsibility for their rashness and inattentiveness. |
Description of changes
Tracking: #197885
python3Packages.torch: include openai-triton in propagatedBuildInputs by default
openai-triton-llvm: init at 14.0.6-f28c006a5895
This makes
openai-triton
andtorchWithRocm
free again.Please note that to test
torch.compile
, you will need to usepython310Packages
because it is not supported with python 3.11 yet. (This is fixed in torch 2.1.0 #259068 pytorch/pytorch#86566)Fixes: #263144
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/
)