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.scikitlearn: apply max_iter patch from scikitlearn master #43483

Merged
merged 1 commit into from Jul 14, 2018
Merged

pythonPackages.scikitlearn: apply max_iter patch from scikitlearn master #43483

merged 1 commit into from Jul 14, 2018

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jul 13, 2018 •
edited

Motivation for this change

See scikit-learn/scikit-learn#10723

This fixes the build of scikitlearn on master and nixos-unstable.

The issue is originally an upstream issue
(see scikit-learn/scikit-learn#10619) which
was fixed on master and was mainly caused by changes to the environment.

Closes #43466

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 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.

@Ma27 Ma27 requested a review from FRidh as a code owner July 13, 2018 13:10
@Ma27
Copy link
Member Author

Ma27 commented Jul 13, 2018

currently running nox-review on my machine. I'll report the result when this is done.

@Ma27
Copy link
Member Author

Ma27 commented Jul 13, 2018

nox-review fails due to pythonPackages.nilearn which seems to be broken on master already.

@dotlambda
Copy link
Member

Shouldn't we apply scikit-learn/scikit-learn@2aba6e2 as a patch instead?

@Ma27
Copy link
Member Author

Ma27 commented Jul 13, 2018

I tried to do that, but it doesn't apply properly.
The current patch was a workaround as I needed scikitlearn, please give me some time tonight then I'll see if I can rebase the patch onto the release.

@Ma27 Ma27 changed the title pythonPackages.scikitlearn: skip test_max_iter test WIP pythonPackages.scikitlearn: skip test_max_iter test Jul 13, 2018
@Ma27
Copy link
Member Author

Ma27 commented Jul 13, 2018

just marked the PR as WIP until I've seen whether it's possible to apply the patch ;)

@Ma27 Ma27 force-pushed the skip-broken-scikitlearn-tests branch from 06bb0c2 to a4174a5 Compare July 13, 2018 20:09
@Ma27 Ma27 changed the title WIP pythonPackages.scikitlearn: skip test_max_iter test pythonPackages.scikitlearn: apply max_iter patch from scikitlearn master Jul 13, 2018
dotlambda
dotlambda reviewed Jul 14, 2018
View reviewed changes
pkgs/development/python-modules/scikitlearn/default.nix Outdated
@@ -14,6 +14,8 @@ buildPythonPackage rec {
sha256 = "5ca0ad32ee04abe0d4ba02c8d89d501b4e5e0304bdf4d45c2e9875a735b323a0";
};

patches = [ ./n_iter-should-be-less-than-max_iter-using-lbgfs.patch ];
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a link to the appropriate commit?

@dotlambda
Copy link
Member

@GrahamcOfBorg build python2.pkgs.scikitlearn python3.pkgs.scikitlearn

@Ma27
pythonPackages.scikitlearn: apply max_iter patch from scikitlearn m…
2644a61 
…aster

See scikit-learn/scikit-learn#10723

This fixes the build of `scikitlearn` on master and nixos-unstable.

The issue is originally an upstream issue
(see scikit-learn/scikit-learn#10619) which
was fixed on master and was mainly caused by changes to the environment.

Closes NixOS#43466
@Ma27 Ma27 force-pushed the skip-broken-scikitlearn-tests branch from a4174a5 to 2644a61 Compare July 14, 2018 11:01
@GrahamcOfBorg
Copy link

Success on x86_64-linux (full log)

Attempted: python2.pkgs.scikitlearn, python3.pkgs.scikitlearn

Partial log (click to expand)

  self.class_log_prior_ = (np.log(self.class_count_) -
.........../nix/store/ryjjlrli4kxgcr8lfbs8fkhzpjiaqwrw-python3.6-scikit-learn-0.19.1/lib/python3.6/site-packages/sklearn/naive_bayes.py:461: RuntimeWarning: divide by zero encountered in log
  self.class_log_prior_ = (np.log(self.class_count_) -
.................................................................
----------------------------------------------------------------------
Ran 8230 tests in 274.656s

OK (SKIP=62)
/nix/store/a3csiki2lfcyxhwk7s84wgrds9svbgw8-python2.7-scikit-learn-0.19.1
/nix/store/ryjjlrli4kxgcr8lfbs8fkhzpjiaqwrw-python3.6-scikit-learn-0.19.1

@dotlambda dotlambda merged commit af17bfd into NixOS:master Jul 14, 2018
@GrahamcOfBorg
Copy link

Failure on aarch64-linux (full log)

Attempted: python2.pkgs.scikitlearn, python3.pkgs.scikitlearn

Partial log (click to expand)

                -5,          0,          0,          0,          0,...
>>  raise AssertionError('\nArrays are not almost equal to 15 decimals\n\n(mismatch 0.793650793651%)\n x: array([[         0,          0,          2,          0,          0,\n                 0,          0,          0,          0,          0,\n                -5,          0,          0,          0,          0,...\n y: array([[         0,          0,          2,          0,          0,\n                 0,          0,          0,          0,          0,\n                -5,          0,          0,          0,          0,...')


----------------------------------------------------------------------
Ran 8230 tests in 1472.104s

FAILED (SKIP=64, failures=1)
builder for '/nix/store/jqy4vvp2rg10rck1rbmnzm7wla7a89iw-python2.7-scikit-learn-0.19.1.drv' failed with exit code 1
error: build of '/nix/store/jqy4vvp2rg10rck1rbmnzm7wla7a89iw-python2.7-scikit-learn-0.19.1.drv', '/nix/store/xyhq27il2b8v2fqfjdkfly9knjnbw60w-python3.6-scikit-learn-0.19.1.drv' failed

@Ma27 Ma27 deleted the skip-broken-scikitlearn-tests branch July 14, 2018 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dotlambda dotlambda dotlambda left review comments

@FRidh FRidh Awaiting requested review from FRidh

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

Successfully merging this pull request may close these issues.

Scikit-learn build failed
3 participants
@Ma27 @dotlambda @GrahamcOfBorg