Skip to content
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.

Sign up for GitHub

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

Jump to bottom

pythonPackages.tensorflow: Hardcode a second search class. #65584

Merged
merged 1 commit into from Jul 31, 2019
Merged

pythonPackages.tensorflow: Hardcode a second search class. #65584

merged 1 commit into from Jul 31, 2019

Conversation

c00w
Copy link
Contributor

@c00w c00w commented Jul 30, 2019 •
edited

It appears without this, libcuda.so.1 is not picked up and nvidia
graphics are broken

Fixes #65515

Motivation for this change

My tensorflow setup doesn't work with head.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nix-review --run "nix-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

@c00w c00w requested a review from FRidh as a code owner July 30, 2019 00:27
@ofborg ofborg bot requested a review from abbradar July 30, 2019 00:38
jonringer
jonringer reviewed Jul 30, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit message: pythonPackages.tensorflow: Hardcode a second search class

@c00w c00w changed the title tensorflow: Hardcode a second search class. pythonPackages.tensorflow: Hardcode a second search class. Jul 30, 2019
@c00w
Copy link
Contributor Author

c00w commented Jul 30, 2019

commit message: pythonPackages.tensorflow: Hardcode a second search class

Done

jonringer
jonringer approved these changes Jul 30, 2019
View reviewed changes
Copy link
Contributor

@jonringer jonringer left a comment •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nix-review passes on NixOS
diff LGTM (although there might be a better way?)
fixes issue mentioned in PR comment

[nix-shell:~/.cache/nix-review/pr-65584]$ nix-shell ./nixpkgs -p python3Packages.tensorflowWithCuda --pure

[nix-shell:~/.cache/nix-review/pr-65584]$ python
Python 3.7.4 (default, Jul  8 2019, 18:31:06)
[GCC 7.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import tensorflow
>>> print(tensorflow.test.is_gpu_available())
2019-07-29 18:14:58.191109: I tensorflow/core/platform/cpu_feature_guard.cc:142] Your CPU supports instructions that this TensorFlow binary was not compiled to use: AVX2 FMA
2019-07-29 18:14:58.233082: I tensorflow/core/platform/profile_utils/cpu_utils.cc:94] CPU Frequency: 3600000000 Hz
2019-07-29 18:14:58.233665: I tensorflow/compiler/xla/service/service.cc:168] XLA service 0x514e30 executing computations on platform Host. Devices:
2019-07-29 18:14:58.233699: I tensorflow/compiler/xla/service/service.cc:175]   StreamExecutor device (0): <undefined>, <undefined>
2019-07-29 18:14:58.236173: I tensorflow/stream_executor/platform/default/dso_loader.cc:42] Successfully opened dynamic library libcuda.so.1
...

@ambrop72
Copy link
Contributor

I suggest to do this instead, which will be more robust:

find -type f \( -name '*.so' -or -name '*.so.*' \) -exec ...

(Personally I would hardcode .so because that expression makes the whole thing unreadable.)

@gloaming
Copy link
Contributor

gloaming commented Jul 30, 2019 •
edited

Yes, you need to account for .so.2 and so on. And there's no need to use extensions.sharedLibrary here since .dylibs and .dlls are not ELF anyway.

@c00w
Copy link
Contributor Author

c00w commented Jul 31, 2019

Switched to a direct so.* match.

@c00w
pythonPackages.tensorflow: Hardcode a second search class.
44bc94e 
It appears without this, libcuda.so.1 is not picked up and nvidia
graphics are broken
@abbradar abbradar merged commit f634a92 into NixOS:master Jul 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonringer jonringer jonringer approved these changes

@FRidh FRidh Awaiting requested review from FRidh

@abbradar abbradar Awaiting requested review from abbradar

Assignees
No one assigned
Labels
6.topic: python 10.rebuild-darwin: 0 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

tensorflow: libcuda not discovered.
5 participants
@c00w @ambrop72 @gloaming @jonringer @abbradar