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.

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

[WIP] pythonPackages.nevergrad: init at 0.3.2 #80424

Closed
wants to merge 1 commit into from

Conversation

@juliendehos
Copy link
Contributor

juliendehos commented Feb 18, 2020

Motivation for this change

Add Nevergrad (a Python gradient-free optimization framework).

Things done

Tested on NixOS and on Nix+Debian10, using the following shell.nix:

with import (fetchTarball "https://github.com/juliendehos/nixpkgs/archive/6afeb1dd1ff5c79b82d0e0ef697fe9cffc434706.tar.gz") {};
(python3.withPackages ( ps: with ps; [ nevergrad ])).env

and with the following example (from Nevergrad docs):

import nevergrad as ng

def square(x):
    return sum((x - .5)**2)

optimizer = ng.optimizers.OnePlusOne(parametrization=2, budget=100)
recommendation = optimizer.minimize(square)
print(recommendation)
  • 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.
@juliendehos juliendehos force-pushed the juliendehos:nevergrad branch from c9a8496 to c38fbff Feb 20, 2020
@juliendehos
Copy link
Contributor Author

juliendehos commented Feb 20, 2020

Thanks for your comments. I've updated the PR and squashed the commits.

Copy link
Contributor

drewrisinger left a comment

Looks pretty good other than small changes. Haven't tried building.

@juliendehos juliendehos force-pushed the juliendehos:nevergrad branch from 4c7be73 to 53f8f93 Mar 26, 2020
@juliendehos juliendehos force-pushed the juliendehos:nevergrad branch from 53f8f93 to 0455d16 Mar 26, 2020

src = fetchFromGitHub {
owner = "facebookresearch";
repo = "nevergrad";

This comment has been minimized.

Copy link
@drewrisinger

drewrisinger Mar 26, 2020

Contributor

You could do repo = pname here but also fine as is.

Copy link
Contributor

drewrisinger left a comment

Build fails (test failures) with nixpkgs-review pr 80424 for python37Packages.nevergrad
Full build log: https://pastebin.com/tvB9ibfc
python38Packages.nevergrad also fails, but due to broken python38Packages.pytorch.

@juliendehos
Copy link
Contributor Author

juliendehos commented Mar 30, 2020

I have the same problem with nix-review but I can't reproduce the problem "manually" in a nix-shell.
There is a new release of nevergrad (0.4.0) so I tried to package it but it now depends on a pre-built binary package (fcmaes) and I don't know how to package that correctly.
So I'm afraid I have to give up with this package.

@drewrisinger
Copy link
Contributor

drewrisinger commented Mar 30, 2020

I have the same problem with nix-review but I can't reproduce the problem "manually" in a nix-shell.

I usually try to build it with e.g. nix-build -A python37Packages.nevergrad. That usually works for me. I've never had luck running the python phases manually from nix-shell.

@drewrisinger
Copy link
Contributor

drewrisinger commented Mar 30, 2020

There is a new release of nevergrad (0.4.0) so I tried to package it but it now depends on a pre-built binary package (fcmaes) and I don't know how to package that correctly.
So I'm afraid I have to give up with this package.

Re fcmaes: I took a brief glance at it GitHub link. You could either package it straight from github (which is a bit harder with installation, see below), or pull it from pypi via fetchPypi (probably easier, looks like it has the pre-built C++ libraries included).

If you choose to install from GitHub, I would just use a regular python package, with the following additions:

  • to buildInputs/nativeBuildInputs: add cmake.
  • to preBuild: run the install.sh script in ./_fcmaes
  • set dontUseCmakeConfigure = true at top level of nix script.

I think that should cover it from a quick glance.

@juliendehos
Copy link
Contributor Author

juliendehos commented Mar 30, 2020

Thanks for the help.
I tried nix-build too but that didn't reproduce the problem either (the tests passed).

I tried to package fcmaes a few days ago and asked for help here : https://discourse.nixos.org/t/build-a-python-package-which-uses-a-c-library/6097/2
It looks like the pre-built lib need to be patched for mkl but I'm not sure how to do that.
Btw, I've just noticed the project seems to include the source code of the lib, so maybe we can build it ourself. I'll try to do that.

@juliendehos juliendehos mentioned this pull request Mar 30, 2020
3 of 10 tasks complete
@juliendehos
Copy link
Contributor Author

juliendehos commented Mar 30, 2020

@drewrisinger I have a package for fcmaes that seems to work, thanks to your help. I'll try to package the latest version of nevergrad and send another PR then.

@drewrisinger
Copy link
Contributor

drewrisinger commented Mar 30, 2020

If it's just those two (nevergrad, fcmaes) as new packages, you could merge them into this one PR and submit together. that allows them to be tested together. could also include the bump to 0.4.0

Copy link
Contributor

drewrisinger left a comment

Comments re pytestCheckHook.

checkInputs = [ pytest ];
checkPhase = "pytest";
Comment on lines +36 to +37

This comment has been minimized.

Copy link
@drewrisinger

drewrisinger Mar 30, 2020

Contributor
Suggested change
checkInputs = [ pytest ];
checkPhase = "pytest";
checkInputs = [ pytestCheckHook];

I suggest using pytestCheckHook, it's slightly cleaner and gives more flexibility in future if you need to disable tests, etc. Not a hard requirement.

, matplotlib
, opencv4
, pandas
, pytest

This comment has been minimized.

Copy link
@drewrisinger

drewrisinger Mar 30, 2020

Contributor
Suggested change
, pytest
, pytestCheckHook
@juliendehos
Copy link
Contributor Author

juliendehos commented Mar 30, 2020

I've already opened a PR for fcmaes. So I will rebase and update this PR when possible (and switch to pytestCheckHook, as suggested).

@veprbl veprbl changed the title pythonPackages.nevergrad: init at 0.3.2 [WIP] pythonPackages.nevergrad: init at 0.3.2 May 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.