Add a Nevergrad Sweeper plugin #430

jrapin · 2020-02-20T18:17:10Z

Motivation

This is a proof of concept for a Nevergrad sweeper pluging.

I won't have time to work much on it so if anyone is willing to take it from there, feel free :D
The current concerns I have:

the documentation should be improved, in particular how to choose an optimizer. The simplest solution is "OnePlusOne" for now. It's incredibly simple and very robust, so I made it the default optimizer.
I don't have ideas for a nice commandline syntax. There are a lot of possible ways to define the parametrization of a problem in nevergrad (probably too many, then again, it's a research framework so we need some flexibility for now :s). I've included a hack to be able to use the full nevergrad syntax, but that won't be very robust :D . For now only choices and standard bounded scalars are supported without the hack, we would at least need a way to define log-distributed scalars.
the launchers seem to only support batches, I suppose there is no way to check if some evaluations have finished so that we could make use of the free workers and launch more evaluations without waiting? That could make the sweep faster, and as far as I know with close to no performance loss.
Minimizing on integers is probably not as robust as minimizing on floats, I'm a bit concerned that anyone writting 0:10 in the commandline would get integers only and may not realize it, maybe the default should be float, and we'd need another way to cast to integers.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Unit tests are included, as well as a runnable example

Related Issues and PRs

Closes #330

lgtm-com · 2020-02-20T18:27:50Z

This pull request introduces 1 alert when merging c5c92df into 4106845 - view on LGTM.com

new alerts:

1 for Unused import

omry · 2020-02-20T18:41:14Z

This is awesome, thanks for taking the time to do this.
I will comment inline, but a few high level comments:

Does nevergrad support maximization as well? it would be better to call this hydra_nevergrad_sweeper and add a configuration option allowing the user to maximize if they choose to.
You have some left over mentions of Example (from example plugin in the docs and the code).
CI is failing. looks like a simple lint issue (you can run ci lint locally with nox -s lint)
I see some deprecation warnings I did not see before from cma

I did not take a close look at the rest of it, but overall it looks good and a great start.

cc @shagunsodhani, when you get some energy after the long Ax stretch - you might want to take this plugin on and polish it a bit.

jrapin · 2020-02-21T09:50:23Z

for maximizing you only need to inverse the output. In nevergrad I tend to limit the number of options so I do not include it directly there. In a configuration tool like this it however makes sense to allow both indeed, so it's now added.
Should be gone now
The lint error is gone, but isort and black are fighting each other on whether to add a skipped line or not when there is a comment within the imports... if you have a good solution against this I am curious, because this is a problem I've stumbled upon several time when trying this combination.
the deprecation warning is from CMA https://github.com/CMA-ES/pycma (our CMA version is just a wrapper around this one), so not directly from nevergrad. Can't do much about it except submit a PR (which I will probably do soon) and encourage them to push a new version.

omry · 2020-02-21T09:53:25Z

add nevergrad to .isort.cfg as a known third party.
I need to do something about it, it's a repeating problem.
Oh, and the released isort has an issue. can't remember if this one.
I am using a version from the repo in my pre-commit:

repos:
  - repo: https://github.com/timothycrosley/isort
    rev: 'c54b3dd'
    hooks:
      - id: isort

sounds good.

By the way, check this up:
omry/omegaconf#152

Coming to OmegaConf 2.0 near you soon. :)

jrapin · 2020-02-21T10:09:00Z

The simplified syntax is more readable and less error prone so that will be nice ;)

I've moved the comment to the top, I think that did the trick (I don't test locally because i'm on a mobile connection and I fear nox is redownloading everything, but it's probably cleverer and has some cache?)
actually CMA is already patched, they just need to provide a new release

omry · 2020-02-21T10:13:25Z

Looking at your config reminded me of that idea.
I liked it so much that I couldn't resist and implemented already.
this is a breaking compatibility for people with dot in their keys, but there is an escape hatch (hehe).
I think it's very much worth it.

Re-download:
Not sure, I suspect it does have some cache.
I don't think you need the comment. (unless I am misunderstanding the purpose).
Once you add nevergrad as an known third party it black and isort should agree on it.

By the way: generally I use typing in Hydra by importing specific things:

from typing import Dict, Any, List

jrapin · 2020-02-21T10:42:51Z

I don't think you need the comment. (unless I am misunderstanding the purpose).

it was there before so I did not dare remove it (google it it seemed it removes some checks)

I use typing in Hydra by importing specific things

I can change that if you want. FYI I was doing the same before in nevergrad, and I'm currently in the process of moving to import typing as tp after I had so many lines with 10 different typing imports, and I was spending a lot of time adding and removing from those lines whenever I was changing code :D

There seems to be some mypy errors. Some are not related to the plugin, but some do, including not finding nevergrad which I should investigate. I never actually checked that the the typing was packaged properly (supposedly it does since there is a py.typed file, but didnt verify).

jrapin · 2020-02-21T14:07:51Z

So, the mypy issue is a bit weird to me. It does look like the py.typed file is correctly packaged (I have a hydra venv where I installed nevergrad from pypi, and the file is in the site-package), and my version of mypy seems to identify it well (I tried to include some non-existing import of nevergrad and it flagged it directly).

Can you see any reason why it would fail to find nevergrad in the CI?

omry · 2020-02-21T16:42:01Z

Comment:
We are talking about # noinspection PyUnresolvedReferences right?
It's for PyCharm. it's supposed to be above this block:

# noinspection PyUnresolvedReferences
from hydra.test_utils.test_utils import TSweepRunner, sweep_runner  # noqa: F401

Import typing as tp:
See what you are saying, let's keep it consistent with the rest of Hydra. I will consider your suggestion to do it this way.

py.typed:
did you try a local repro with nox -s lint?

Looking at the lint in the noxfile, it does not install the plugins (so the dependencies are not installed as well).
I am a bit confused as to why other plugins like joblib are also not facing the same issue.

jrapin · 2020-02-21T18:08:36Z

did you try a local repro with nox -s lint?

I'm still avoiding it given the mobile connection

I am a bit confused as to why other plugins like joblib are also not facing the same issue.

Got it :D from joblib import Parallel, delayed # type: ignore
Basically, I suppose this imports them as Any and then everything works. I can do the same trick but that's probably not an effective typing :D Plugins (or at least their dependencies) should be installed (or use mypy --ignore-missing-imports or whitelist ignored imports in mypy.ini). Let me know what you prefer.

See what you are saying, let's keep it consistent with the rest of Hydra. I will consider your suggestion to do it this way.

That's your call, I'll update it (took me a year to change to import typing as tp, let's talk about it in a year :D)

We are talking about # noinspection PyUnresolvedReferences right? It's for PyCharm. it's supposed to be above this block.

Yes. Does it have to be exactly the line above? If so then I'm stuck with the isort/black incompatibility. If not I keep it on the very top and everything is fine

plugins/hydra_nevergrad_sweeper/example/config.yaml

jrapin · 2020-03-06T17:14:49Z

While testing the mac situation, I ran into this failure which seems logical:
https://app.circleci.com/jobs/github/facebookresearch/hydra/10892

I've added a seed to avoid flakiness. I would have expected this should not happen often at all (I hoped never), too bad :(

...ra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/conf/hydra/sweeper/nevergrad.yaml

plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/core.py

plugins/hydra_nevergrad_sweeper/setup.py

plugins/hydra_nevergrad_sweeper/tests/test_nevergrad_sweeper_plugin.py

omry · 2020-03-06T19:06:53Z

While testing the mac situation, I ran into this failure which seems logical:
https://app.circleci.com/jobs/github/facebookresearch/hydra/10892

I've added a seed to avoid flakiness. I would have expected this should not happen often at all (I hoped never), too bad :(

It's not too bad. I think some randomness is those algorithms by default is actually a good thing. locking the test with a seed seems like a good solution to me.

website/README.md

plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/core.py

website/docs/plugins/nevergrad_sweeper.md

omry · 2020-03-08T23:09:03Z

website/docs/plugins/nevergrad_sweeper.md

+The output of a run looks like:
+The initialization of the sweep and the first 5 evaluations (out of 100) look like this:


Two lines with:
colon seems weird:

removed one of them which should not have been there

website/docs/plugins/nevergrad_sweeper.md

plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/__init__.py

Co-Authored-By: Omry Yadan <[email protected]>

plugins/hydra_nevergrad_sweeper/setup.py

omry · 2020-03-09T17:02:58Z

website/docs/plugins/nevergrad_sweeper.md

 ```

-The application `main` returns a float which we want to minimimize, the minimum is 0 and reached for:
+The the function decorated with `@hydra.main()` returns a float which we want to minimize, the minimum is 0 and reached for:


Should mention how to maximize somewhere in the readme.
Maybe as a commented out line in the config node above.

added to the conf

jrapin

I feel it's mostly done now.
There must be some corner cases but I expect that will be solved when we agree on the inputs

jrapin · 2020-03-10T09:05:30Z

website/docs/plugins/nevergrad_sweeper.md

 ```

-The application `main` returns a float which we want to minimimize, the minimum is 0 and reached for:
+The the function decorated with `@hydra.main()` returns a float which we want to minimize, the minimum is 0 and reached for:


added to the conf

omry · 2020-03-10T18:18:21Z

Landing it (without even taking final final look).
We will continue to evolve and update it as the common interface is maturing.

Thanks for all your work on this @jrapin.

jrapin added 13 commits February 20, 2020 09:53

Add basic example

83acfaf

Updated names

e9d6f2a

Add tests

79fc30a

Working example

857f5af

Updated

a02f31c

Fix

6472de8

typo

32975be

Active test

f565a53

Improved test

7e9f31d

Update doc

f169ad7

Add docstring

3fddf81

More docstrings

d934529

remove_pretty

c5c92df

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 20, 2020

jrapin added 4 commits February 21, 2020 09:30

Rename

e7aa866

Allow maximization

f90dc0a

Allow maximization

d961ad8

black

df713bd

move comment

832e541

Use hydra typing import pattern and add nevergrad seeding

1488d5b

config API

bb27f5d

omry reviewed Mar 6, 2020

View reviewed changes

plugins/hydra_nevergrad_sweeper/example/config.yaml Outdated Show resolved Hide resolved

Update documentation

d5131b2

omry reviewed Mar 6, 2020

View reviewed changes

website/README.md Outdated Show resolved Hide resolved

omry reviewed Mar 6, 2020

View reviewed changes

plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/core.py Show resolved Hide resolved

jrapin added 2 commits March 6, 2020 22:44

nits

aa2f8a7

new specs for logs and ints

ef4f503

omry reviewed Mar 8, 2020

View reviewed changes

plugins/hydra_nevergrad_sweeper/hydra_plugins/hydra_nevergrad_sweeper/__init__.py Outdated Show resolved Hide resolved

jrapin and others added 8 commits March 9, 2020 13:57

Update website/docs/plugins/nevergrad_sweeper.md

fbb1adf

Co-Authored-By: Omry Yadan <[email protected]>

Avoid importing twice

79fdb29

Update doc

c0c5724

nit

e9c4d26

New nevergrad version

ad08a18

Merge branch 'master' into minimizer

b75a3b5

badges

de98f1d

badges

f938b26

omry reviewed Mar 9, 2020

View reviewed changes

plugins/hydra_nevergrad_sweeper/setup.py Show resolved Hide resolved

omry reviewed Mar 9, 2020

View reviewed changes

shagunsodhani mentioned this pull request Mar 10, 2020

Specifying parameter type (via cmd) for Ax/Nevergrad Plugins #437

Closed

nits

3d877e8

jrapin commented Mar 10, 2020

View reviewed changes

yangshun requested review from yangshun and removed request for yangshun March 10, 2020 09:08

omry merged commit 0bfb4f4 into facebookresearch:master Mar 10, 2020

jrapin deleted the minimizer branch March 11, 2020 13:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a Nevergrad Sweeper plugin #430

Add a Nevergrad Sweeper plugin #430

jrapin commented Feb 20, 2020

lgtm-com bot commented Feb 20, 2020

omry commented Feb 20, 2020

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020 •

edited

Loading

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020

jrapin commented Feb 21, 2020 •

edited

Loading

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020

jrapin commented Feb 21, 2020 •

edited

Loading

jrapin commented Mar 6, 2020

omry commented Mar 6, 2020

omry Mar 8, 2020

jrapin Mar 9, 2020

omry Mar 9, 2020

jrapin Mar 10, 2020

jrapin left a comment

jrapin Mar 10, 2020

omry commented Mar 10, 2020

		The output of a run looks like:
		The initialization of the sweep and the first 5 evaluations (out of 100) look like this:

Add a Nevergrad Sweeper plugin #430

Add a Nevergrad Sweeper plugin #430

Conversation

jrapin commented Feb 20, 2020

Motivation

Have you read the Contributing Guidelines on pull requests?

Test Plan

Related Issues and PRs

lgtm-com bot commented Feb 20, 2020

omry commented Feb 20, 2020

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020 • edited Loading

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020

jrapin commented Feb 21, 2020 • edited Loading

jrapin commented Feb 21, 2020

omry commented Feb 21, 2020

jrapin commented Feb 21, 2020 • edited Loading

jrapin commented Mar 6, 2020

omry commented Mar 6, 2020

omry Mar 8, 2020

Choose a reason for hiding this comment

jrapin Mar 9, 2020

Choose a reason for hiding this comment

omry Mar 9, 2020

Choose a reason for hiding this comment

jrapin Mar 10, 2020

Choose a reason for hiding this comment

jrapin left a comment

Choose a reason for hiding this comment

jrapin Mar 10, 2020

Choose a reason for hiding this comment

omry commented Mar 10, 2020

omry commented Feb 21, 2020 •

edited

Loading

jrapin commented Feb 21, 2020 •

edited

Loading

jrapin commented Feb 21, 2020 •

edited

Loading