Skip to content

pythonPackages.osqp: remove mkl dependency#84124

Merged
bhipple merged 1 commit intoNixOS:masterfrom
drewrisinger:dr-pr-py-osqp-remove-mkl
Apr 3, 2020
Merged

pythonPackages.osqp: remove mkl dependency#84124
bhipple merged 1 commit intoNixOS:masterfrom
drewrisinger:dr-pr-py-osqp-remove-mkl

Conversation

@drewrisinger
Copy link
Copy Markdown
Contributor

Motivation for this change

Removing mkl allows this to be built in Hydra (mkl is unfree-ish license), and it seems the mkl dependency is somewhat optional given downstream packages build cleanly when removing mkl.
Also add explicit numpy/scipy dependency.
Cleanup commit slightly for formatting.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • 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 nixpkgs-review --run "nixpkgs-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.

Also add scipy.
Cleanup commit slightly for formatting.
Removing mkl allows this to be built in Hydra (mkl = unfree),
and it seems the mkl dependency is somewhat optional given downstream
packages build cleanly when removing mkl.
@drewrisinger
Copy link
Copy Markdown
Contributor Author

@GrahamcOfBorg build python37Packages.osqp python37Packages.cvxpy python38Packages.osqp python38Packages.cvxpy

@ofborg ofborg bot added 6.topic: python Python is a high-level, general-purpose programming language. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. labels Apr 2, 2020
@bhipple
Copy link
Copy Markdown
Contributor

bhipple commented Apr 2, 2020

CC @matthewbauer, who is looking at making this type of thing parameterizable in #83888

@drewrisinger
Copy link
Copy Markdown
Contributor Author

drewrisinger commented Apr 2, 2020 via email

@matthewbauer
Copy link
Copy Markdown
Member

This case is a little different because osqp doesn't use MKL for BLAS, but MKL PARDISO (https://software.intel.com/en-us/mkl-developer-reference-fortran-intel-mkl-pardiso-parallel-direct-sparse-solver-interface) which is unique to MKL.

Anyway, it looks like this library just needs MKL to test the MKL functionality. Since osqp just uses dlopen, you can always enable MKL support through LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(nix-build '<nixpkgs>' -A mkl)/lib.

@drewrisinger
Copy link
Copy Markdown
Contributor Author

This case is a little different because osqp doesn't use MKL for BLAS, but MKL PARDISO (https://software.intel.com/en-us/mkl-developer-reference-fortran-intel-mkl-pardiso-parallel-direct-sparse-solver-interface) which is unique to MKL.

Anyway, it looks like this library just needs MKL to test the MKL functionality. Since osqp just uses dlopen, you can always enable MKL support through LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$(nix-build '<nixpkgs>' -A mkl)/lib.

Ack, thanks for the good tips. I wasn't familiar with what MKL PARDISO did. pythonPackages.osqp does just use MKL PARDISO for testing PARDISO functionality AFAICT, it probably also enables some runtime functionality but I view that as a downstream thing. I prefer for build purposes to have no unfree dependencies so as much as possible can be built/cached by Hydra.

@drewrisinger
Copy link
Copy Markdown
Contributor Author

@GrahamcOfBorg build python37Packages.qiskit-aer python37Packages.qiskit-ignis
@GrahamcOfBorg build python38Packages.qiskit-aer python38Packages.qiskit-ignis

@drewrisinger
Copy link
Copy Markdown
Contributor Author

Result of nixpkgs-review pr 84124

9 package built:
python27Packages.osqp python37Packages.cvxpy python37Packages.osqp python37Packages.qiskit-aer python37Packages.qiskit-ignis python38Packages.cvxpy python38Packages.osqp python38Packages.qiskit-aer python38Packages.qiskit-ignis

Copy link
Copy Markdown
Contributor

@bhipple bhipple left a comment

Choose a reason for hiding this comment

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

Result of nixpkgs-review pr 84124 1

9 package built: - python27Packages.osqp - python37Packages.cvxpy - python37Packages.osqp - python37Packages.qiskit-aer - python37Packages.qiskit-ignis - python38Packages.cvxpy - python38Packages.osqp - python38Packages.qiskit-aer - python38Packages.qiskit-ignis

@bhipple bhipple merged commit c5313eb into NixOS:master Apr 3, 2020
@bhipple
Copy link
Copy Markdown
Contributor

bhipple commented Apr 3, 2020

Can you send a 20.03 backport PR as well?

@drewrisinger drewrisinger deleted the dr-pr-py-osqp-remove-mkl branch April 3, 2020 22:09
@drewrisinger
Copy link
Copy Markdown
Contributor Author

drewrisinger commented Apr 3, 2020

@bhipple python3Packages.osqp isn't in release-20.03.

@bhipple
Copy link
Copy Markdown
Contributor

bhipple commented Apr 3, 2020

Ah ok, my bad! For some reason I thought we'd had this packaged for longer, but I was thinking of another related pkg.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: python Python is a high-level, general-purpose programming language. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants