mkl: include Intel's libiomp.so in the MKL RPM unpack #52543
Conversation
|
CC @FRidh @costrouc @markuskowa @veprbl @matthewbauer I've tested this with numpy, numexpr, scipy and pandas, and it appears to work without issue, and without requiring env vars or setup hooks, which means usages outside of nix builds work. @tbenst I have not tried it with This takes the runtime package up from 655MB to 663MB, which is relatively minor; since it's not distributed by Hydra it should not cost the common nix infra anything, and for users it'll mean the package works out of the box, while still allowing for the LLVM |
Since Intel's default openmp implementation is available in the same src tarball, we can just include it in the package. This means that `mkl` now "just works" without any environment variables, fragile setup-hooks, or forced propagation. Since the openmp implementation is only needed at runtime (and for test cases), users can substitute a different one if they prefer by exporting it with `LD_PRELOAD`, which is how Intel recommends handling this. If they do not do so, `libiomp.so` lives next to `libmkl_rt.so` and thus will be in the RPATH as a sane default. Since this still comes from the same src tarball, we can ship it without losing the fixed-output derivation; likewise, since Hydra is not building or caching these, shipping these proprietary packages costs no bandwidth for the nix community.
bhipple
force-pushed
the
feature/mkl-iomp
branch
from
9352dee
to
c28495c
Compare
| @@ -66,8 +60,8 @@ stdenvNoCC.mkDerivation rec { | |||
| outputHashAlgo = "sha256"; | |||
| outputHashMode = "recursive"; | |||
| outputHash = if stdenvNoCC.isDarwin | |||
| then "1224dln7n8px1rk8biiggf77wjhxh8mzw0hd8zlyjm8i6j8w7i12" | |||
| else "0d8ai0wi8drp071acqkm1wv6vyg12010y843y56zzi1pql81xqvx"; | |||
| then "0000000000000000000000000000000000000000000000000000" | |||
There was a problem hiding this comment.
@smaret can you run nix-build -A mkl and let me know what the Darwin hash is, as you did last time?
#47182 (comment)
BTW, is there a way for me to cross compile to Darwin and compute the hash on my own, or is that only available for FOSS platforms?
| preBuild = '' | ||
| rm site.cfg | ||
| ln -s ${numpy.cfg} site.cfg | ||
| export LD_LIBRARY_PATH=${llvmPackages.openmp}/lib |
There was a problem hiding this comment.
This kind of stuff worked fine during the nix build and in nix shells, but fails immediately if you try to install and use it; this whole package as a whole has gotten significantly simpler now that we provide a sane default and just stick with Intel's recommendation to use a runtime LD_PRELOAD if you want something else, since it's not used at build-time and is binary compatible with the LLVM openmp.
| @@ -71,8 +71,6 @@ in buildPythonPackage rec { | |||
| inherit blasImplementation cfg; | |||
| }; | |||
|
|
|||
| doCheck = blasImplementation != "mkl"; | |||
There was a problem hiding this comment.
All of the tests pass just fine now.
| @@ -43,16 +30,23 @@ stdenvNoCC.mkDerivation rec { | |||
| '' else '' | |||
| rpmextract rpm/intel-mkl-common-c-${date}-${rel}-${date}-${rel}.noarch.rpm | |||
| rpmextract rpm/intel-mkl-core-rt-${date}-${rel}-${date}-${rel}.x86_64.rpm | |||
| rpmextract rpm/intel-openmp-19.0.0-${rel}-19.0.0-${rel}.x86_64.rpm | |||
There was a problem hiding this comment.
This is the meat of the change: we're just unpacking this extra RPM and putting it next to libmkl_rt.so so it has its dependencies available. Since this comes in the same src tarball, we can maintain this package as an efficient fixed-output derivation while still keeping the nix expression relatively simple.
|
Thanks for the quick review and merge @FRidh! While this is ready-to-go for Linux, someone with a Mac needs to find out the output hash for this: https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/libraries/science/math/mkl/default.nix#L63 |
Since Intel's default openmp implementation is available in the same src
tarball, we can just include it in the package. This means that
mklnow "justworks" without any environment variables, fragile setup-hooks, or forced
propagation.
Since the openmp implementation is only needed at runtime (and for test cases),
users can substitute a different one if they prefer by exporting it with
LD_PRELOAD, which is how Intel recommends handling this. If they do not do so,libiomp.solives next tolibmkl_rt.soand thus will be in the RPATH as asane default.
Since this still comes from the same src tarball, we can ship it without losing
the fixed-output derivation; likewise, since Hydra is not building or caching
these, shipping these proprietary packages costs no bandwidth for the nix
community.
Motivation for this change
Fixes #48876 with a cleaner, more robust implementation than the one initially proposed in #48926
Things done
sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)nix path-info -Sbefore and after)