-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
python312Packages.tensorflow: use tensorflow-bin by default #345658
base: master
Are you sure you want to change the base?
Conversation
184346b
to
e80cf0a
Compare
|
cc @NixOS/cuda-maintainers, #342112 (where this was discussed some) FWIW, I have a local work‐in‐progress branch to bump the TensorFlow version and I think it looks feasible. However I’m not a TensorFlow expert and have no intrinsic motivation to maintain it so I can’t promise I will see it through to completion. This seems like a reasonable fallback. |
The main blocker for this PR is the failure of
|
I can see how this patch is tempting given the crude tools we have, but I think having |
Thanks for the recommendation. |
e9d3c97
to
804211c
Compare
|
No, sorry for being obscure. Of course I acknowledge I couldn't block this change as an emergency measure based only on nonsensical purist considerations... maybe in future in situations like this we agree on a deadline for merging? @zeuner I'm sooo happy to hear this:) |
If we satisfy TensorFlow dependencies with our binary package – if it is our blessed TensorFlow – then it seems like it would be confusing and bad for |
Indeed, after the creation of this PR, @zeuner has been resuming his effort on #272838. |
It's a valid point, but this already happens in many places, e.g. with Side note: if we introduced an |
I think there’s a difference between overriding the default version for some packages, and overriding the variant used for everything, as we would be doing here. This is just a redirecting of the alias |
Thanks
Well that's where we diverge: I haven't been considering |
After discussion on Matrix, I think we should move forward with this for now. I hope @zeuner’s work bears fruit and we can have an up‐to‐date TensorFlow source build for 25.05, but as it stands, the outdated TensorFlow source build is keeping alive an end‐of-life Bazel, a CUDA that was scheduled for removal in #342112, and an LLVM version that would otherwise likely be fairly easy to drop. I’m watching #272838 with interest, but the version it would update to would still reference compilers that are causing fuss now. I would prefer to merge this and mark the source build as broken so that we can drop the old CUDAs and compilers early in the cycle and benefit from the resulting clean‐up, with the hopes of restoring and switching back to the source build on an up‐to‐date version during the 25.05 cycle, rather than leaving the situation unresolved until we potentially have to rush that before release, having spent effort on keeping the old compilers going before then. That said, of course, I’d appreciate @zeuner’s input since they’re the one doing the vital work here :) |
Things done
It has been several months that our tensorflow build has been stuck on version 2.13.0 (released Jul. 5 2023).
Also, tensorflow 2.13.0 is not supported on python 3.12, which makes all its dependents incompatible too.
To this date, python 3.11 is still part of our pair of supported versions in nixpkgs, but this will not last forever.
As no one seems to have both the time and skills to tackle this tricky update (#WeLoveBazel), I propose to use our up-to-date binary package in the meantime.
cc @SomeoneSerge @zeuner @mweinelt @abbradar
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.