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.mkl-service: init at 2.1.0 #64932

Merged
merged 1 commit into from Jul 25, 2019
Merged

pythonPackages.mkl-service: init at 2.1.0 #64932

merged 1 commit into from Jul 25, 2019

Conversation

bhpdt
Copy link

@bhpdt bhpdt commented Jul 16, 2019 •
edited

Motivation for this change

This provides a python interface for controlling mkl runtime behavior.

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.

@bhpdt bhpdt requested a review from FRidh as a code owner July 16, 2019 18:09
@bhipple
Copy link
Contributor

bhipple commented Jul 16, 2019

@GrahamcOfBorg build python2Packages.mkl-service python3Packages.mkl-service

FRidh
FRidh reviewed Jul 16, 2019
View reviewed changes
pkgs/development/python-modules/mkl-service/default.nix Outdated
MKLROOT = mkl;

checkInputs = [ nose ];
propagatedBuildInputs = [ cython mkl six ];
Copy link
Member

Choose a reason for hiding this comment

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

Is cython needed during runtime...?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch, it's just needed at buid-time; updated accordingly!

@bhpdt
Copy link
Author

bhpdt commented Jul 16, 2019

The bot isn't building it since the dependency mkl has a proprietary license, but if you check it out locally you can see all 73 test cases pass in both python2 and python3 flavors.

CC @markuskowa @smaret @costrouc

bhpdt
bhpdt commented Jul 16, 2019
View reviewed changes
pkgs/development/python-modules/mkl-service/default.nix
sha256 = "1bnpgx629rxqf0yhn0jn68ypj3dqv6njc3981j1g8j8rsm5lycrn";
};

MKLROOT = mkl;
Copy link
Author

Choose a reason for hiding this comment

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

This is needed so that setup.py picks up the location of the mkl package:
https://github.com/IntelPython/mkl-service/blob/master/setup.py#L67

@bhpdt bhpdt changed the title mkl-service: init at 2.1.0 pythonPackages.mkl-service: init at 2.1.0 Jul 16, 2019
@smaret
Copy link
Member

smaret commented Jul 17, 2019 •
edited

Built successfully on Darwin with nix-review pr and tested with the small example code on mkl-service homepage.

@costrouc
Copy link
Member

It built successfully on x86 linux and ran the same code example as @smaret

@risicle
Copy link
Contributor

risicle commented Jul 17, 2019

Probably better to squash your commits - deep git history doesn't need to know you didn't get the commit perfect the first time.

@nixos-discourse
Copy link

This pull request has been mentioned on Nix community. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/39

@markuskowa markuskowa merged commit 88d9f77 into NixOS:master Jul 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@FRidh FRidh FRidh left review comments

Assignees
No one assigned
Labels
6.topic: python 8.has: package (new) 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
8 participants
@bhpdt @bhipple @smaret @costrouc @risicle @nixos-discourse @FRidh @markuskowa