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

scalapack: switch to openblas and add test #49199

Merged
merged 1 commit into from Nov 5, 2018
Merged

scalapack: switch to openblas and add test #49199

merged 1 commit into from Nov 5, 2018

Conversation

markuskowa
Copy link
Member

Motivation for this change

Use openblas instead of the reference implementations of BLAS and LAPACK.

CC @costrouc for testing and comments

Things done
  • change from blas,lapack to openblas
  • add check phase
  • enable parallel building
  • move cmake to nativeBuildInputs
  • fix license (it's a BSD 3-clause license)
  • 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 nox --run "nox-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)
  • Fits CONTRIBUTING.md.

@markuskowa
scalapack: switch to openblas + add test
41a579a 
* change from blas,lapack to openblas
* add check phase
* enable parallel building
* fix license
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: scalapack

Partial log (click to expand)

-- Installing: /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2/lib/cmake/scalapack-2.0.2/scalapack-targets.cmake
-- Installing: /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2/lib/cmake/scalapack-2.0.2/scalapack-targets-release.cmake
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2
shrinking /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2/lib/libscalapack.so
strip is /nix/store/vcc4svb8gy29g4pam2zja6llkbcwsyiq-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2/lib
patching script interpreter paths in /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2
checking for references to /build in /nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2...
/nix/store/8zb8xmzirq0bqqlhjslr0g4yykmkwx3f-scalapack-2.0.2

@GrahamcOfBorg
Copy link

Success on aarch64-linux (full log)

Attempted: scalapack

Partial log (click to expand)

-- Installing: /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2/lib/cmake/scalapack-2.0.2/scalapack-targets.cmake
-- Installing: /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2/lib/cmake/scalapack-2.0.2/scalapack-targets-release.cmake
post-installation fixup
shrinking RPATHs of ELF executables and libraries in /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2
shrinking /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2/lib/libscalapack.so
strip is /nix/store/p9akxn2sfy4wkhqdqa3li97pc6jaz3r1-binutils-2.30/bin/strip
stripping (with command strip and flags -S) in /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2/lib
patching script interpreter paths in /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2
checking for references to /build in /nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2...
/nix/store/1dn6kqgg4d6as2l5kvnx8pfy44kfsdqm-scalapack-2.0.2

@costrouc
Copy link
Member

I forsee that we are going to want to add an mkl derivation to this as well. Could you change openblasCompat to blas and supply blas=openblasCompat similar to how it is done with numpy in python? Not sure if this is the recommended way but I am sure that we will want to allow several blas implementations

@markuskowa
Copy link
Member Author

@costrouc I had this in my initial version (see https://github.com/markuskowa/NixOS-QChem/blob/master/scalapack/default.nix). However, at least for now I decided against it for two reasons. (1) It requires some if-then-else statement to get the library string right, adding complexity to the derivation. (2) If I remember correctly MKL aready provides a version of ScaLAPACK. We probably want to use the MKL provided version instead.

If you decide to add MKL support to this derivation later, changing the names is no big effort.

@costrouc
Copy link
Member

costrouc commented Oct 27, 2018 •
edited

Just read the mkl docs on scalapack. Definitely agree with the simplicity argument and how mkl already implements the routines. PR looks good to me thanks!

@markuskowa markuskowa merged commit bf8c031 into NixOS:master Nov 5, 2018
@markuskowa markuskowa deleted the fet-scalapack branch November 7, 2018 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ttuegel ttuegel ttuegel approved these changes

Assignees
No one assigned
Labels
10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@markuskowa @GrahamcOfBorg @costrouc @ttuegel