Replace InstrumentedFunction by ExperimentFunction (different API) #421

jrapin · 2019-12-30T11:30:41Z

Types of changes

Docs change / refactoring / dependency upgrade
Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)

Motivation and Context / Related issue

Improve reproducibility of random functions, simplifies API, solves bugs

How Has This Been Tested (if it applies)

Checklist

The documentation is up-to-date with the changes I made.
I have read the CONTRIBUTING document and completed the CLA (see CONTRIBUTING).
All tests passed, and additional code has been covered with new tests.

jrapin · 2019-12-30T21:12:48Z

@teytaud this solves the reproducibility issues
Eventually (= as soon as I have time to update the code) InstrumentedFunction will disappear in favor of ExperimentFunction.
The main differences are:

the initialization takes 1 function and 1 instrumentation (instead of 1 function and possibly many variables)
__call__ directly forwards the arguments to the function, without converting to data space (while before it would take a np.ndarray from the data space
it has a copy method, which makes sure we can a new version with new random values we can seed (this is what makes everything work)

I also renamed instrumentation to parametrization ahead of the other PRs

jrapin · 2019-12-30T21:16:08Z

TODO: update doc

jrapin

This is now ready to review.

jrapin · 2020-01-02T12:57:36Z

nevergrad/benchmark/frozenexperiments.py

@@ -35,7 +35,7 @@ def repeated_basic(seed: Optional[int] = None) -> Iterator[Experiment]:
    optims: List[Union[str, OptimizerFamily]] = ["OnePlusOne", optimizers.DifferentialEvolution()]
    for _ in range(5):
        for optim in optims:
-            yield Experiment(function.duplicate(), optimizer=optim, num_workers=2, budget=4, seed=next(seedg))


No more duplication here, since the function is now duplicated whenever it is actually running. This way seeding will be much safer (we seed right before duplication)

jrapin · 2020-01-02T12:58:42Z

nevergrad/benchmark/test_execution.py

        return x + y

    # pylint: disable=unused-argument
-    def get_postponing_delay(self, args: Tuple[Any, ...], kwargs: Dict[str, Any], value: float) -> float:


As a reminder, this is used to control "pseudo/mock" timing during the experiments (to compare steady and batch modes for instance)

jrapin · 2020-01-02T12:59:54Z

nevergrad/benchmark/test_experiments.py

@@ -21,8 +21,7 @@ def test_experiments_registry(name: str, maker: Callable[[], Iterator[experiment
    with patch("shutil.which", return_value="here"):  # do not check for missing packages
        with datasets.mocked_data():  # mock mlda data that should be downloaded
            check_maker(maker)  # this is to extract the function for reuse if other external packages need it
-        if name not in {"realworld_oneshot", "mlda", "mldaas", "realworld", "powersystems", "powersystemsbig",
-                        "fastgames", "bigfastgames"}:
+        if name not in {"realworld_oneshot", "mlda", "mldaas", "realworld"}:


seeding now works now (except mlda, but that's only because we cant download the data)

jrapin · 2020-01-02T13:00:59Z

nevergrad/benchmark/xpbase.py

 from ..optimization.optimizerlib import registry as optimizer_registry  # import from optimizerlib so as to fill it
 from . import execution

 registry = decorators.Registry[Callable[..., Iterator['Experiment']]]()


-class IFuncWrapper(execution.PostponedObject):


No more need for this wrapper, which is good :)

jrapin · 2020-01-02T13:05:56Z

nevergrad/benchmark/xpbase.py

        self.function = function
        # Conjecture on the noise level.
-        if not self.function.instrumentation.probably_noisy:
-            if hasattr(self.function, "_parameters") and "noise_level" in self.function._parameters and self.function._parameters["noise_level"] > 0:  # type: ignore


@teytaud the probably_noisy tag is now dealt with directly in ArtificialFunction, which is safer

jrapin · 2020-01-02T13:09:35Z

nevergrad/functions/photonics/core.py

@@ -124,7 +125,8 @@ def __init__(self, name: str, dimension: int, transform: str = "tanh") -> None:
        self._func = inst.CommandFunction(["octave-cli", "--no-gui", "--no-history", "--norc", "--quiet", "--no-window-system", path.name],
                                          cwd=path.parent, verbose=False,
                                          env=dict(os.environ, OMP_NUM_THREADS="1", OPENBLAS_NUM_THREADS="1"))
-        super().__init__(self._compute, *_make_instrumentation(name=name, dimension=dimension, transform=transform).args)
+        super().__init__(self._compute, _make_instrumentation(name=name, dimension=dimension, transform=transform))
+        self.register_initialization(name=name, dimension=dimension, transform=transform)


just to make it clear once: register_initialization keeps a record of the initialization parameters so that the "copy" method can reinstantiate a new object easily

jrapin · 2020-01-02T13:11:46Z

nevergrad/functions/rl/agents.py

-        self._descriptors.update(num_repetitions=self.runner.num_repetitions, instrumentation="")
+        super().__init__(self.compute, self.agent.instrumentation.copy().with_name(""))
+        self.register_initialization(agent=agent, env_runner=env_runner, reward_postprocessing=reward_postprocessing)
+        self._descriptors.update(num_repetitions=self.runner.num_repetitions, archi=self.agent.module.__class__.__name__)


I've added the archi descriptor here. It takes the module class name which is Perceptron or DenseNet. This will replace mono and multi which were manually added in the experiments (which does not work anymore -> descriptors should preferably be automatic to avoid errors)

jrapin · 2020-01-02T13:13:53Z

nevergrad/functions/stsp/core.py

-        self.y = self.instrumentation.random_state.normal(size=self.dimension)
+        self.register_initialization(dimension=dimension)
+        self.order = np.arange(0, self.dimension)
+        self.x = self.parametrization.random_state.normal(size=self.dimension)


just to make it clear once, instrumentation becomes parametrization in ExperimentFunction for forward compatibility with future updates (see #391 ), but the underlying instance is still an Instrumentation. At some point in the near future, this will also be the case for optimizers (but after the merge of new parametrization framework).

jrapin · 2020-01-02T13:15:44Z

nevergrad/functions/utils.py

@@ -27,30 +26,3 @@ def __call__(self, x: np.ndarray) -> np.ndarray:
        if self.rotation_matrix is not None:
            y = self.rotation_matrix.dot(y)
        return y
-
-
-class PostponedObject(abc.ABC):


Both those classes are not necessary anymore since the methods are directly included in ExperimentFunction

teytaud

For the moment I am doing tests by running NGO and checking if it correctly identifies the type of functions and everything is fine. I keep testing.

jrapin · 2020-01-06T12:28:08Z

Reviewed with @teytaud, ready to go

Replace InstrumentedFunction by ParametrizedFunction (different API)

c11fc7a

facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Dec 30, 2019

minor_edit

209d7f8

jrapin changed the title ~~Replace InstrumentedFunction by ParametrizedFunction (different API)~~ [WIP] Replace InstrumentedFunction by ParametrizedFunction (different API) Dec 30, 2019

jrapin mentioned this pull request Dec 30, 2019

[WIP] Prepare new parametrized function #403

Closed

7 tasks

jrapin added 9 commits December 30, 2019 13:37

Merge branch 'master' into paramfunc

5772278

Reproducible power system

a6bf893

Removed prints

d0b0a6d

Get rid of PostponedFunction and NoisyFunction

fe5a4e1

Update xp example

a2cfd10

Solve most issues

f9211db

Updated PackedFunction

96f14ce

Debug

56b5793

Make everything seedable

4921f2e

jrapin changed the title ~~[WIP] Replace InstrumentedFunction by ParametrizedFunction (different API)~~ Replace InstrumentedFunction by ParametrizedFunction (different API) Dec 30, 2019

jrapin added 2 commits December 30, 2019 21:47

Update name

a623e78

Update to parametrization

fa4408e

jrapin requested a review from teytaud December 30, 2019 21:13

jrapin added Difficulty: Medium Priority: Critical Type: Breaking changes Breaking changes to the API, to be notified in changelogs Type: Bug Something isn't working labels Dec 30, 2019

jrapin added 6 commits December 31, 2019 12:30

Update naming

5971603

Update import paths

c68e9b3

Convert aritificial function

4646bf9

Convert aritificial function

5dc7f45

Revert bad commit

fdc75b8

Converted other functions

35be5cb

jrapin added 7 commits January 2, 2020 13:30

Deprecate old InstrumentedFunction and restructure

d8d1307

Update documentation

82ac90d

Remove unnecessary copies

cece2bd

Better doc

3e1dfec

Updated Changelog

5b2972a

smaller diff

1f109ad

Deal with probably_noisy tag where it matters

208bfee

jrapin commented Jan 2, 2020

View reviewed changes

Remove artificial function copy

b69b2bb

jrapin mentioned this pull request Jan 3, 2020

Merge master with ExperimentFunction #432

Merged

7 tasks

jrapin changed the title ~~Replace InstrumentedFunction by ParametrizedFunction (different API)~~ Replace InstrumentedFunction by ExperimentFunction (different API) Jan 3, 2020

teytaud reviewed Jan 6, 2020

View reviewed changes

jrapin merged commit 594cf2c into master Jan 6, 2020

jrapin deleted the paramfunc branch January 6, 2020 12:28

jrapin mentioned this pull request Jan 27, 2020

Simplify Instrumented function #321

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace InstrumentedFunction by ExperimentFunction (different API) #421

Replace InstrumentedFunction by ExperimentFunction (different API) #421

jrapin commented Dec 30, 2019 •

edited

Loading

jrapin commented Dec 30, 2019 •

edited

Loading

jrapin commented Dec 30, 2019

jrapin left a comment

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

jrapin Jan 2, 2020

teytaud left a comment

jrapin commented Jan 6, 2020

Replace InstrumentedFunction by ExperimentFunction (different API) #421

Replace InstrumentedFunction by ExperimentFunction (different API) #421

Conversation

jrapin commented Dec 30, 2019 • edited Loading

Types of changes

Motivation and Context / Related issue

How Has This Been Tested (if it applies)

Checklist

jrapin commented Dec 30, 2019 • edited Loading

jrapin commented Dec 30, 2019

jrapin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

teytaud left a comment

Choose a reason for hiding this comment

jrapin commented Jan 6, 2020

jrapin commented Dec 30, 2019 •

edited

Loading

jrapin commented Dec 30, 2019 •

edited

Loading