Add adapters for experiment version control #83

bouthilx · 2018-05-16T03:06:34Z

Depends on #86

Why:

Experiment branches because of changes in the configuration or
the user's code. This make experiments incompatible with one another
unless we define adapters such that a branched experiment B can access
trials from parent experiment A.

How:

Adapters have two main methods, forward and backward. The method forward
defines how trials from parent experiment A are filtered or adapted to
be compatible with experiment B. Modifications are only applied at
execution time and are not saved anywhere in the database. Adapters only
provides a view on an experiment.

There is adapters for

Dimension addition
Dimension deletion
Dimension renaming
Change of dimension prior
Change of algorithm
Change of code
Combining different adapters

Adapters all have a to_dict method which provides sufficient
information to rebuild the adapter. This is to facilitate save of
adapters in a database and retrieval.

Adapters can be build using the factory class Adapter(**kwargs) or using
Adapter.build(list_of_dicts).

codecov-io · 2018-05-17T18:07:59Z

Codecov Report

Merging #83 into feature/evc will increase coverage by 0.57%.
The diff coverage is 99.64%.

@@               Coverage Diff               @@
##           feature/evc      #83      +/-   ##
===============================================
+ Coverage        95.22%   95.79%   +0.57%     
===============================================
  Files               45       48       +3     
  Lines             3725     4284     +559     
  Branches           295      331      +36     
===============================================
+ Hits              3547     4104     +557     
- Misses             143      144       +1     
- Partials            35       36       +1

Impacted Files	Coverage Δ
src/orion/core/worker/trial.py	`100% <100%> (ø)`	⬆️
tests/unittests/core/test_trial.py	`100% <100%> (ø)`	⬆️
src/orion/core/evc/__init__.py	`100% <100%> (ø)`
tests/unittests/core/evc/test_adapters.py	`100% <100%> (ø)`
src/orion/core/evc/adapters.py	`98.7% <98.7%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fdc7b3...15490fa. Read the comment docs.

tsirif

Beautiful code man, I really enjoyed it!

tsirif · 2018-05-17T19:08:08Z

src/orion/core/evc/adapters.py

+                                   (self.param.name, trial))
+
+            adapted_trial = copy.deepcopy(trial)
+            adapted_trial.params.append(self.param)


Q: Is the default value part of Trial.Param?

Well, a Trial.Param instance is basically a dimension.sample() turned into a param. So, if we have a default value, Trial.Param is a very convenient and natural way to encapsulate such default value. Furthermore, for DimensionAddition and DimensionDeletion we need to add a Trial.Param object to each trials, so using such encapsulation for default value is very very convenient.

I agree. This means that default_value should be a slotted attribute of Trial.Param?

I don't think it should. The default_value is encapsulated as the value of Trial.Param. You could have either

Trial.Param(dimension.name, dimension.type, dimension.sample())

or

Trial.Param(dimension.name, dimension.type, dimension.default_value)

We don't need to know if it was sampled or if it is the default value. It simply is the value of the param.

Oh I see now. I didn't see it. 😝

tsirif · 2018-05-17T19:13:52Z

src/orion/core/evc/adapters.py

+        return ret
+
+
+class DimensionDeletion(DimensionAddition):


Oh this is what you meant you could have done with a composition. Yessss

I'll rework it a bit.

tsirif · 2018-05-17T20:41:07Z

src/orion/core/worker/trial.py

@@ -106,6 +106,10 @@ def to_dict(self):
                )
            return ret

+        def __eq__(self, other):


This should have been done earlier. It is useful fo the hash_name property as well. Shall I right a __hash__ and change the implementation of the hash_name? I think I should. This will be in a future PR.

Yes sure if there is hashing that is certainly related to eq.

tsirif · 2018-05-17T20:49:33Z

src/orion/core/worker/trial.py

@@ -132,13 +136,15 @@ class Result(Value):
        function or of an 'constraint' expression.
        """

+        __slots__ = ('name', '_type', 'value')


Oh yes. Sorry for that. The tuple defining the slots can be still in Trial.Value. What needs to be done is to declare __slots__ = () in Trial.Param and Trial.Result, so that a namespace __dict__ won't be created for Trial.Param or Trial.Result. An object instantiated from the inherited classes will be able only have access to the non-blank namespace defined by the base class.

Oh! I will try that.

tsirif · 2018-05-17T20:49:44Z

src/orion/core/worker/trial.py

        allowed_types = ('objective', 'constraint', 'gradient')

    class Param(Value):
        """Types for a `Param` can be either an integer (discrete value),
        floating precision numerical or a categorical expression (e.g. a string).
        """

+        __slots__ = ('name', '_type', 'value')


tsirif · 2018-05-17T20:51:01Z

tests/unittests/core/test_utils.py

@@ -0,0 +1,37 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+"""Test base fonctionnalities of :mod:`orion.core.utils`."""


Oh typo here, bad me

Bad me? I did the typo, no?

bouthilx · 2018-05-18T00:38:04Z

@tsirif I'm glad you enjoyed reading it ;) Thanks for your support 😛

I think I addressed everything you mentioned now.

Why: Experiment branches because of changes in the configuration or the user's code. This make experiments incompatible with one another unless we define adapters such that a branched experiment B can access trials from parent experiment A. How: Adapters have two main methods, forward and backward. The method forward defines how trials from parent experiment A are filtered or adapted to be compatible with experiment B. Modifications are only applied at execution time and are not saved anywhere in the database. Adapters only provides a *view* on an experiment. There is adapters for * Dimension addition * Dimension deletion * Dimension renaming * Change of dimension prior * Change of algorithm * Change of code * Combining different adapters Adapters all have a `to_dict` method which provides sufficient information to rebuild the adapter. This is to facilitate save of adapters in a database and retrieval. Adapters can be build using the factory class Adapter(**kwargs) or using Adapter.build(list_of_dicts).

Why: Classes do not inherit __slots__, it needs to be redefined in each children. The parent class Value's __init__ depends on the combination of kwargs parsing and __slots__ to raise AttributeError on invalid arguments. Since Param and Result do not inherint __slots__, they fail to raise an AttributeError when passed bad attributes.

Why: We need to compare params very often inside adapters. Also Param objects with identical name, type and values should be considered as identical.

corneauf · 2018-05-18T15:37:56Z

Can this be merged?

bouthilx · 2018-05-18T15:46:53Z

@DatCorno @tsirif It's up to you. You both approve the PR in its current state?

tsirif · 2018-05-18T15:50:14Z

I am waiting for corno's input. I approved this message.

…

On Fri, May 18, 2018, 11:46 Xavier Bouthillier ***@***.***> wrote: @DatCorno <https://github.com/DatCorno> @tsirif <https://github.com/tsirif> It's up to you. You both approve the PR in its current state? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#83 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AD5VAUM4_BV8ci6imsQFBihcHRUJ3_QRks5tzuzugaJpZM4UAk81> .

corneauf · 2018-05-18T16:05:39Z

From what I can see everything's good.

bouthilx changed the base branch from develop to feature/evc May 16, 2018 03:07

bouthilx changed the title ~~Adapter - First draft~~ WIP: Adapter - First draft May 16, 2018

bouthilx changed the title ~~WIP: Adapter - First draft~~ [WIP] Adapter - First draft May 16, 2018

bouthilx force-pushed the feature/evc-adapters branch from 4b32985 to fa4d2a8 Compare May 17, 2018 18:00

bouthilx changed the title ~~[WIP] Adapter - First draft~~ Add adapters for experiment version control May 17, 2018

bouthilx requested review from tsirif and corneauf May 17, 2018 18:03

tsirif reviewed May 17, 2018

View reviewed changes

This was referenced May 18, 2018

Default value should be a slotted attribute of Trial.Param #88

Closed

Trial.Param should be hashable #89

Closed

bouthilx force-pushed the feature/evc-adapters branch from fa4d2a8 to ccd41cf Compare May 18, 2018 00:30

bouthilx changed the base branch from feature/evc to develop May 18, 2018 00:32

bouthilx changed the base branch from develop to feature/evc May 18, 2018 00:32

bouthilx force-pushed the feature/evc-adapters branch from ccd41cf to a0dcf1b Compare May 18, 2018 00:36

bouthilx added 3 commits May 17, 2018 20:46

Add __eq__ to Trial.Param

15490fa

Why: We need to compare params very often inside adapters. Also Param objects with identical name, type and values should be considered as identical.

bouthilx force-pushed the feature/evc-adapters branch from a0dcf1b to 15490fa Compare May 18, 2018 00:46

corneauf approved these changes May 18, 2018

View reviewed changes

bouthilx merged commit 74f12a9 into Epistimio:feature/evc May 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add adapters for experiment version control #83

Add adapters for experiment version control #83

bouthilx commented May 16, 2018 •

edited

Loading

codecov-io commented May 17, 2018 •

edited

Loading

tsirif left a comment

tsirif May 17, 2018

bouthilx May 17, 2018

tsirif May 18, 2018

tsirif May 18, 2018

bouthilx May 18, 2018

tsirif May 18, 2018

tsirif May 17, 2018

bouthilx May 17, 2018

tsirif May 17, 2018

bouthilx May 17, 2018

tsirif May 18, 2018

tsirif May 17, 2018

bouthilx May 17, 2018

tsirif May 17, 2018

tsirif May 17, 2018

bouthilx May 18, 2018

bouthilx commented May 18, 2018

corneauf commented May 18, 2018

bouthilx commented May 18, 2018

tsirif commented May 18, 2018 via email

corneauf commented May 18, 2018

Add adapters for experiment version control #83

Add adapters for experiment version control #83

Conversation

bouthilx commented May 16, 2018 • edited Loading

codecov-io commented May 17, 2018 • edited Loading

Codecov Report

tsirif 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

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

bouthilx commented May 18, 2018

corneauf commented May 18, 2018

bouthilx commented May 18, 2018

tsirif commented May 18, 2018 via email

corneauf commented May 18, 2018

bouthilx commented May 16, 2018 •

edited

Loading

codecov-io commented May 17, 2018 •

edited

Loading