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

Add configspace conversion #832

Merged
merged 52 commits into from
Jul 29, 2022
Merged
Show file tree
Hide file tree
Changes from 50 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
3883fc2
Add configspace conversion
Delaunay Mar 16, 2022
e2dbae9
Add precision
Delaunay Mar 17, 2022
fa2663d
Isort tweak
Delaunay Mar 17, 2022
4c6811d
Remove S type from visitor
Delaunay Mar 17, 2022
442c95e
Fix typo
Delaunay Mar 17, 2022
061708d
Add default_value case
Delaunay Mar 17, 2022
acd5a46
Add ConfigSpace as an optional dependency
Delaunay Mar 17, 2022
12c656b
Add singledispatch to make to_orionspace work for any registered types
Delaunay Mar 18, 2022
746970b
Add docstrings
Delaunay Mar 18, 2022
3f77f71
-
Delaunay Mar 18, 2022
674c84e
Document typevar
Delaunay Mar 18, 2022
6bf7518
__all__ without T
Delaunay Mar 18, 2022
a14d8c6
Revert
Delaunay Mar 18, 2022
64555ed
Update src/orion/algo/space/configspace.py
Delaunay Mar 19, 2022
c7b5a46
Try to fix doc generation bypassing typing
Delaunay Mar 21, 2022
c9136fd
Merge branch 'issue_586' of github.com:Delaunay/orion into issue_586
Delaunay Mar 21, 2022
18f7395
Mock uninstalled imports, handle py3.6 for configspace
Delaunay Mar 22, 2022
72002cc
Fix tests for 3.6
Delaunay Mar 24, 2022
c083bff
Merge branch 'develop' of github.com:Epistimio/orion into issue_586
Delaunay Mar 24, 2022
8c7dac9
Remove python3.6 hacks
Delaunay Mar 24, 2022
a5a4ac6
Use single dispatch
Delaunay Mar 24, 2022
e4f6c56
Missing return
Delaunay Mar 24, 2022
5b8a2c0
Update src/orion/algo/space/configspace.py
Delaunay Mar 25, 2022
107e5bd
Add unsupported ConfigSpace priors
Delaunay Mar 25, 2022
47216cc
Merge branch 'issue_586' of github.com:Delaunay/orion into issue_586
Delaunay Mar 25, 2022
fd58aa1
Add unsupported prior
Delaunay Mar 28, 2022
8e8b47b
-
Delaunay Mar 28, 2022
a8f0425
-
Delaunay Mar 28, 2022
8abd4fb
-
Delaunay Mar 28, 2022
eb57c58
Update src/orion/algo/space/configspace.py
Delaunay Mar 28, 2022
fec66c3
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
ebc9886
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
0087ef8
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
016a6d2
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
5ff9d8e
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
59bdad2
Update src/orion/algo/space/configspace.py
Delaunay Mar 28, 2022
0709f8d
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
17af4ce
Update src/orion/algo/space/__init__.py
Delaunay Mar 28, 2022
38c1ae5
-
Delaunay Mar 28, 2022
d818079
-
Delaunay Mar 28, 2022
534ab5e
-
Delaunay Mar 28, 2022
192f31c
Update src/orion/algo/space/configspace.py
Delaunay Mar 28, 2022
a104255
Update src/orion/algo/space/configspace.py
Delaunay Mar 28, 2022
f60858a
Update src/orion/algo/space/configspace.py
Delaunay Mar 28, 2022
37c580e
Merge branch 'develop' of github.com:Epistimio/orion into issue_586
Delaunay Mar 28, 2022
65b6f7d
-
Delaunay Mar 28, 2022
a14a47b
Update tests/unittests/algo/test_configspace.py
Delaunay Apr 2, 2022
0a99146
Merge branch 'develop' of github.com:Epistimio/orion into issue_586
Delaunay Apr 11, 2022
9de07ad
-
Delaunay Apr 12, 2022
5648f03
-
Delaunay Apr 12, 2022
06adba3
Merge branch 'develop' into issue_586
bouthilx Jul 27, 2022
bd9680f
black
bouthilx Jul 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ sphinx_rtd_theme
sphinxcontrib.httpdomain
sphinx-autoapi
sphinx_gallery
sphinx-autodoc-typehints
numpydoc
plotly
matplotlib
Expand Down
1 change: 1 addition & 0 deletions docs/src/code/algo/space.rst
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
Space search
============


.. automodule:: orion.algo.space
:members:
3 changes: 2 additions & 1 deletion docs/src/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@

# -- Autodoc configuration -----------------------------------------------

autodoc_mock_imports = ["_version", "utils._appdirs"]
autodoc_mock_imports = ["_version", "utils._appdirs", "torch"]

# -- Gallery configuration -----------------------------------------------

Expand Down Expand Up @@ -302,6 +302,7 @@
"orion.core.utils.tree.T",
"orion.core.utils.tree.NodeType",
"orion.core.utils.tree.Self",
"orion.algo.space.T",
"Self",
"AlgoType",
"T",
Expand Down
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
"dask": ["dask[complete]"],
"track": ["track @ git+https://github.com/Delaunay/track"],
"profet": ["emukit", "GPy", "torch", "pybnn"],
"configspace": ["ConfigSpace"]
}
extras_require["all"] = list(set(sum(extras_require.values(), [])))

Expand Down
73 changes: 73 additions & 0 deletions src/orion/algo/space.py → src/orion/algo/space/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,13 @@
unless noted otherwise!

"""
from __future__ import annotations

import copy
Delaunay marked this conversation as resolved.
Show resolved Hide resolved
import logging
import numbers
from functools import singledispatch
from typing import Any, Generic, TypeVar

import numpy
from scipy.stats import distributions
Expand Down Expand Up @@ -67,6 +71,64 @@ def __repr__(self):
return "..."


def _to_snake_case(name: str) -> str:
"""Transform a class name ``MyClassName`` to snakecase ``my_class_name``"""
frags = []

frag = []
for c in name:
if c.isupper() and frag:
frags.append("".join(frag).lower())
frag = []

frag.append(c)

if frag:
frags.append("".join(frag).lower())

return "_".join(frags)


T = TypeVar("T")


class SpaceConverter(Generic[T]):
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't really see the point of having a SpaceConverter. Is it to force an external library to provide conversion functions functions for the given types of spaces?

Also, this generic signature isn't really respected by the ToConfigSpace class, right? Since it doesn't support Fidelity-type spaces, it returns None in that case, and so it becomes a SpaceConverter[Optional[HyperParameter]]!

Copy link
Collaborator

Choose a reason for hiding this comment

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

bump

Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

Users can look at the visitor and see clearly all the methods he is expected to override to implement a valid feature.

I guess this answers my first question. However, it's clear from the fidelity example that you actually don't want to enforce that every type of dimension has to be supported by subclasses. Therefore, it's not actually an interface, if all its members are optional. It's only use, as far as I can see, is to be a list of names of methods that "can" be implemented by subclasses. It also doesn't have any state.

NB: Visitor are more than just for the conversion they can be used to implement any new features outside of Dimensions
Users could implement their own sample/serialization/... using a Visitor.

At this point, why not just let the user code create a function that does what they want to do, e.g. serialize / print / do something for the types of Dimension of Orion they want to support?

Copy link
Member

Choose a reason for hiding this comment

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

To be fair the fidelity is the only exception. I don't see how the conversions could be usable if they do not support all other type of dimensions. I think the abstract class is the clearest way to express the interface expected for contributors. The only thing needed would be to clarify the exception for the fidelity.

"""SpaceConverter iterates over an Orion search space.
This can be used to implement new features for ``orion.algo.space.Space``
outside of Orion's code base.

"""

def convert_dimension(self, dimension: Dimension) -> T:
"""Call the dimension conversion handler"""
return getattr(self, _to_snake_case(type(dimension).__name__))(dimension)
Comment on lines +101 to +103
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is essentially a singledispatch callable in disguise: You have handlers for each type below, and you dispatch based on the "type" of the argument, indirectly.
However, having an implicit dependency on the name of the dimension classes is very brittle, and hard to keep track of. Could you make this more explicit somehow?
I also don't think this should be inside a class, since there isn't (and most likely won't be) any state.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Also, what happens if a new type of dimension is added, and there is no method with that name? This will raise an AttributeError, but you'd ideally like a NotImplementedError with an informative message.

Copy link
Member

Choose a reason for hiding this comment

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

If we make it an abstract class then non-implemented methods would already raise an informative message during code development. It's true there is no state so making it a class isn't super useful for functionality's sake but I think it is nevertheless useful to make the interface more clear. The other solution would be to have a module for these functions only (without the methods for the conversion the other way around).

In any case, we should make it such that if the dev forgets to write some methods it fails with a clear message at execution or some tests automatically detects this.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, this should use the property type to work with transformed dimensions. Looks like we should also have tests for converting transformed spaces.

Suggested change
def convert_dimension(self, dimension: Dimension) -> T:
"""Call the dimension conversion handler"""
return getattr(self, _to_snake_case(type(dimension).__name__))(dimension)
def convert_dimension(self, dimension: Dimension) -> T:
"""Call the dimension conversion handler"""
return getattr(self, _to_snake_case(dimension.type))(dimension)

Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

As long as:

  • we don't dispatch based on the names of classes, and
  • the solution is consistent with itself and the base class,
    then I don't have a problem with using an ABC and marking all the methods with @abstractmethod.

Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

It's true there is no state so making it a class isn't super useful for functionality's sake but I think it is nevertheless useful to make the interface more clear. The other solution would be to have a module for these functions only (without the methods for the conversion the other way around).

Since the abstract base class would only have static methods, this is actually a good use-case for Protocols:

from typing_extensions import Protocol

T = TypeVar("T")

class SpaceConverter(Protocol[T]):
     integer: Callable[[Integer], T]
     real: Callable[[Real], T]
     fidelity: Callable[[Fidelity], T | None]

This way, if you have a module like so:

# nevergrad/conversion.py (or something like that)

def integer(dim: Integer) -> NeverGradStuff:
     ...
    
def real(dim: Real) -> NeverGradReal:
    ... 
    
def fidelity(dim: Fidelity -> None:
    return None # Nevergrad doesn't support Fidelity dimensions.

The procol can be used just like the abstract base class, and has the benefit of allowing type checkers to also check against a module:

from nevergrad import conversion

space_converter: SpaceConverter[NeverGradStuff] = conversion    # Passes, since all the required methods are defined.


def dimension(self, dim: Dimension) -> T:
"""Called when the dimension does not have a decicated handler"""
pass

def real(self, dim: Real) -> T:
"""Called by real dimension"""
pass

def integer(self, dim: Integer) -> T:
"""Called by integer dimension"""
pass

def categorical(self, dim: Categorical) -> T:
"""Called by categorical dimension"""
pass

def fidelity(self, dim: Fidelity) -> T:
"""Called by fidelity dimension"""
pass

def space(self, space: Space) -> None:
"""Iterate through a research space and visit each dimensions"""
for _, dim in space.items():
self.visit(dim)


class Dimension:
"""Base class for search space dimensions.

Expand Down Expand Up @@ -1092,3 +1154,14 @@ def cardinality(self):
for dim in self.values():
capacities *= dim.cardinality
return capacities


@singledispatch
def to_orionspace(space: Any) -> Space:
"""Convert a third party search space into an Orion compatible space

Raises
------
NotImplementedError if no conversion was registered
"""
raise NotImplementedError()
262 changes: 262 additions & 0 deletions src/orion/algo/space/configspace.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,262 @@
from __future__ import annotations

from functools import singledispatch
Delaunay marked this conversation as resolved.
Show resolved Hide resolved
from math import log10

from orion.algo.space import (
Categorical,
Dimension,
Fidelity,
Integer,
Real,
Space,
SpaceConverter,
to_orionspace,
)

try:
from ConfigSpace import ConfigurationSpace
from ConfigSpace.hyperparameters import (
CategoricalHyperparameter,
FloatHyperparameter,
Hyperparameter,
IntegerHyperparameter,
NormalFloatHyperparameter,
NormalIntegerHyperparameter,
UniformFloatHyperparameter,
UniformIntegerHyperparameter,
)

IMPORT_ERROR = None

except ImportError as err:
IMPORT_ERROR = err

class DummyType:
"""Dummy type for type hints"""

pass

IntegerHyperparameter = DummyType
FloatHyperparameter = DummyType
ConfigurationSpace = DummyType
Hyperparameter = DummyType
UniformFloatHyperparameter = DummyType
NormalFloatHyperparameter = DummyType
UniformIntegerHyperparameter = DummyType
NormalIntegerHyperparameter = DummyType
CategoricalHyperparameter = DummyType


class UnsupportedPrior(Exception):
pass


def _qantization(dim: Dimension) -> float:
"""Convert precision to the quantization factor"""
if dim.precision:
return 10 ** (-dim.precision)
return None


class ToConfigSpace(SpaceConverter[Hyperparameter]):
Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

I believe this should be removed, see below.

"""Convert an Orion space into a configspace"""

def __init__(self) -> None:
if IMPORT_ERROR is not None:
raise IMPORT_ERROR

def dimension(self, dim: Dimension) -> None:
"""Raise an error if the visitor is called on an abstract class"""
raise NotImplementedError()

def real(self, dim: Real) -> FloatHyperparameter:
"""Convert a real dimension into a configspace equivalent"""
if dim.prior_name in ("reciprocal", "uniform"):
a, b = dim._args

return UniformFloatHyperparameter(
name=dim.name,
lower=a,
upper=b,
default_value=dim.default_value,
q=_qantization(dim),
log=dim.prior_name == "reciprocal",
)

if dim.prior_name in ("normal", "norm"):
a, b = dim._args

kwargs = dict(
name=dim.name,
mu=a,
sigma=b,
default_value=dim.default_value,
q=_qantization(dim),
log=False,
lower=dim.low if hasattr(dim, "low") else None,
upper=dim.high if hasattr(dim, "high") else None,
)

return NormalFloatHyperparameter(**kwargs)

raise UnsupportedPrior(f'Prior "{dim.prior_name}" is not supported')

def integer(self, dim: Integer) -> IntegerHyperparameter:
"""Convert a integer dimension into a configspace equivalent"""
if dim.prior_name in ("int_uniform", "int_reciprocal"):
a, b = dim._args

return UniformIntegerHyperparameter(
name=dim.name,
lower=a,
upper=b,
default_value=dim.default_value,
q=_qantization(dim),
log=dim.prior_name == "int_reciprocal",
)

if dim.prior_name in ("int_norm", "normal"):
a, b = dim._args

kwargs = dict(
name=dim.name,
mu=a,
sigma=b,
default_value=dim.default_value,
q=_qantization(dim),
log=False,
lower=dim.low if hasattr(dim, "low") else None,
upper=dim.high if hasattr(dim, "high") else None,
)

return NormalIntegerHyperparameter(**kwargs)

raise UnsupportedPrior(f'Prior "{dim.prior_name}" is not supported')

def categorical(self, dim: Categorical) -> CategoricalHyperparameter:
"""Convert a categorical dimension into a configspace equivalent"""
return CategoricalHyperparameter(
name=dim.name,
choices=dim.categories,
weights=dim._probs,
)

def fidelity(self, dim: Fidelity) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is incoherent with the type signature of the SpaceConverter[HyperParameter] class.

Copy link
Collaborator

Choose a reason for hiding this comment

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

bump

"""Ignores fidelity dimension as configspace does not have an equivalent"""
return None

def space(self, space: Space) -> ConfigurationSpace:
Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

I suggest moving this logic to a Space handler of to_configspace:

@to_configspace.register(Space)
<this code>

"""Convert orion space to configspace"""
cspace = ConfigurationSpace()
dims = []

for _, dim in space.items():
bouthilx marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for _, dim in space.items():
for dim in space.values():

cdim = self.convert_dimension(dim)

if cdim:
dims.append(cdim)

cspace.add_hyperparameters(dims)
return cspace


Copy link
Collaborator

@lebrice lebrice Apr 13, 2022

Choose a reason for hiding this comment

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

I strongly suggest making this a singedispatch callable like you did with to_oriondim, and cutting-pasting the methods of the ToConfigSpace class into handler functions for each supported dimension type below.

Suggested change
@singledispatch

def to_configspace(space: Space) -> ConfigurationSpace:
"""Convert orion space to configspace

Notes
-----
``ConfigurationSpace`` will set its own default values
if not set inside ``Space``

"""
conversion = ToConfigSpace()
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should be the "root" of the singledispatch callable, and just return a NotImplementedError for the given argument.

return conversion.space(space)


@singledispatch
def to_oriondim(dim: Hyperparameter) -> Dimension:
"""Convert a config space hyperparameter to an orion dimension"""
raise NotImplementedError(f"Dimension {dim} is not supported by Orion")


@to_oriondim.register
def _from_categorical(dim: CategoricalHyperparameter) -> Categorical:
"""Builds a categorical dimension from a categorical hyperparameter"""
choices = {k: w for k, w in zip(dim.choices, dim.probabilities)}
return Categorical(dim.name, choices)


@to_oriondim.register(UniformIntegerHyperparameter)
@to_oriondim.register(UniformFloatHyperparameter)
def _from_uniform(dim: Hyperparameter) -> Integer | Real:
"""Builds a uniform dimension from a uniform hyperparameter"""

klass = Integer
args = []
kwargs = dict(
# NOTE: Config space always has a config value
# so orion-space would get it as well
default_value=dim.default_value
)

if isinstance(dim, UniformFloatHyperparameter):
klass = Real
else:
kwargs["precision"] = int(-log10(dim.q)) if dim.q else 4

dist = "uniform"
args.append(dim.lower)
args.append(dim.upper)

if dim.log:
dist = "reciprocal"

return klass(dim.name, dist, *args, **kwargs)


@to_oriondim.register(NormalFloatHyperparameter)
@to_oriondim.register(NormalIntegerHyperparameter)
def _from_normal(dim: Hyperparameter) -> Integer | Real:
"""Builds a normal dimension from a normal hyperparameter"""

klass = Integer
args = []
kwargs = dict(
# NOTE: Config space always has a config value
# so orion-space would get it as well
default_value=dim.default_value
)

if isinstance(dim, NormalFloatHyperparameter):
klass = Real
else:
kwargs["precision"] = int(-log10(dim.q)) if dim.q else 4

dist = "norm"
args.append(dim.mu)
args.append(dim.sigma)

if dim.lower:
kwargs["low"] = dim.lower
kwargs["high"] = dim.upper

return klass(dim.name, dist, *args, **kwargs)


@to_orionspace.register
def configspace_to_orionspace(cspace: ConfigurationSpace) -> Space:
"""Convert from orion space to configspace

Notes
-----
``ConfigurationSpace`` will set default values for each dimensions of ``Space``

"""
space = Space()

for cdim in cspace.get_hyperparameters_dict().values():
odim = to_oriondim(cdim)
space.register(odim)

return space
Loading