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.

Sign up for GitHub

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

Jump to bottom

Enable Optimizer state and Multi-Fidelity passthrough in SMAC #751

Closed
wants to merge 26 commits into from
Closed

Enable Optimizer state and Multi-Fidelity passthrough in SMAC #751

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
7f8a43b
minimal implementation of mutli-fidelity
jsfreischuetz May 22, 2024
8afb5f0
revert changes
jsfreischuetz May 23, 2024
08575af
revert
jsfreischuetz May 23, 2024
fcfca53
fix minor bug with logging
jsfreischuetz Jun 1, 2024
838c1db
undo formatting
jsfreischuetz Jun 1, 2024
7533b4e
Update mlos_core/mlos_core/optimizers/optimizer.py
jsfreischuetz Jun 3, 2024
3904020
merge
jsfreischuetz Jun 3, 2024
4ffff6c
Merge branch 'microsoft-main' into multifidleity
jsfreischuetz Jun 3, 2024
b7de120
merge
jsfreischuetz Jun 3, 2024
7278994
add checks back to optimizer
jsfreischuetz Jun 4, 2024
c79294a
add checks back
jsfreischuetz Jun 4, 2024
048269c
add checks back
jsfreischuetz Jun 4, 2024
019192a
update name of context to metadata, and add readme
jsfreischuetz Jun 5, 2024
88d63c1
update tests to also use correct terminology
jsfreischuetz Jun 5, 2024
4e36f28
Update mlos_core/mlos_core/optimizers/bayesian_optimizers/smac_optimi…
jsfreischuetz Jun 6, 2024
3326ac9
Update mlos_core/mlos_core/optimizers/README.md
jsfreischuetz Jun 6, 2024
2399d3e
Update mlos_core/mlos_core/optimizers/README.md
jsfreischuetz Jun 6, 2024
1f210b5
Add context back to the register interface
jsfreischuetz Jun 6, 2024
87a5af9
Merge branch 'main' into multifidleity
motus Jun 7, 2024
48af70f
Apply suggestions from code review
bpkroth Jun 12, 2024
cd8deff
Merge branch 'main' into multifidleity
motus Jun 12, 2024
271a79b
Update mlos_core/mlos_core/optimizers/optimizer.py
jsfreischuetz Jun 12, 2024
98c7398
Update mlos_core/mlos_core/optimizers/optimizer.py
jsfreischuetz Jun 12, 2024
9726410
Update mlos_core/mlos_core/optimizers/optimizer.py
jsfreischuetz Jun 12, 2024
bf4602b
Update mlos_core/mlos_core/optimizers/optimizer.py
jsfreischuetz Jun 12, 2024
8d2a894
fix comments for python
jsfreischuetz Jun 14, 2024
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
6 changes: 4 additions & 2 deletions .vscode/settings.json
View file
Open in desktop
jsfreischuetz marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
// See Also:
// - https://github.com/microsoft/vscode/issues/2809#issuecomment-1544387883
// - mlos_bench/config/schemas/README.md

{
"fileMatch": [
"mlos_bench/mlos_bench/tests/config/schemas/environments/test-cases/**/*.jsonc",
Expand Down Expand Up @@ -139,5 +138,8 @@
"--log-level=DEBUG",
"."
],
"python.testing.unittestEnabled": false
"python.testing.unittestEnabled": false,
"cSpell.words": [
"SOBOL"
]
jsfreischuetz marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 1 addition & 1 deletion mlos_bench/mlos_bench/optimizers/mlos_core_optimizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ def suggest(self) -> TunableGroups:
tunables = super().suggest()
if self._start_with_defaults:
_LOG.info("Use default values for the first trial")
df_config = self._opt.suggest(defaults=self._start_with_defaults)
df_config, _ = self._opt.suggest(defaults=self._start_with_defaults)
self._start_with_defaults = False
_LOG.info("Iteration %d :: Suggest:\n%s", self._iter, df_config)
return tunables.assign(
Expand Down
348 changes: 282 additions & 66 deletions mlos_core/mlos_core/optimizers/bayesian_optimizers/smac_optimizer.py
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions mlos_core/mlos_core/optimizers/flaml_optimizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Contains the FlamlOptimizer class.
"""

from typing import Dict, List, NamedTuple, Optional, Union
from typing import Dict, List, NamedTuple, Optional, Tuple, Union
from warnings import warn

import ConfigSpace
Expand Down Expand Up @@ -108,7 +108,9 @@ def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,

self.evaluated_samples[cs_config] = EvaluatedSample(config=config.to_dict(), score=score)

def _suggest(self, context: Optional[pd.DataFrame] = None) -> pd.DataFrame:
def _suggest(
self, context: Optional[pd.DataFrame] = None
) -> Tuple[pd.DataFrame, Optional[pd.DataFrame]]:
"""Suggests a new configuration.

Sampled at random using ConfigSpace.
Expand All @@ -126,7 +128,7 @@ def _suggest(self, context: Optional[pd.DataFrame] = None) -> pd.DataFrame:
if context is not None:
warn(f"Not Implemented: Ignoring context {list(context.columns)}", UserWarning)
config: dict = self._get_next_config()
return pd.DataFrame(config, index=[0])
return pd.DataFrame(config, index=[0]), None

def register_pending(self, configurations: pd.DataFrame,
context: Optional[pd.DataFrame] = None) -> None:
Expand Down
37 changes: 29 additions & 8 deletions mlos_core/mlos_core/optimizers/optimizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class BaseOptimizer(metaclass=ABCMeta):

def __init__(self, *,
parameter_space: ConfigSpace.ConfigurationSpace,
optimization_targets: List[str],
optimization_targets: str | List[str] | None = None,
jsfreischuetz marked this conversation as resolved.
Show resolved Hide resolved
space_adapter: Optional[BaseSpaceAdapter] = None):
"""
Create a new instance of the base optimizer.
Expand All @@ -52,6 +52,8 @@ def __init__(self, *,
self._observations: List[Tuple[pd.DataFrame, pd.DataFrame, Optional[pd.DataFrame]]] = []
self._has_context: Optional[bool] = None
self._pending_observations: List[Tuple[pd.DataFrame, Optional[pd.DataFrame]]] = []
self.delayed_config: Optional[pd.DataFrame] = None
self.delayed_context: Optional[pd.DataFrame] = None

def __repr__(self) -> str:
return f"{self.__class__.__name__}(space_adapter={self.space_adapter})"
Expand All @@ -76,8 +78,10 @@ def register(self, configurations: pd.DataFrame, scores: pd.DataFrame,
Not Yet Implemented.
"""
# Do some input validation.
assert set(scores.columns) == set(self._optimization_targets), \
"Mismatched optimization targets."
if type(self._optimization_targets) is str:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if type(self._optimization_targets) is str:
assert self._optimization_targets, "Missing or invalid optimization targets"
if type(self._optimization_targets) is str:

Copy link
Contributor

Choose a reason for hiding this comment

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

Also assert not empty (see also comment above about accepting None)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this makes sense given my comment above

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, but separate PR for that one please

assert self._optimization_targets in scores.columns, "Mismatched optimization targets."
if type(self._optimization_targets) is list:
assert set(scores.columns) >= set(self._optimization_targets), "Mismatched optimization targets."
assert self._has_context is None or self._has_context ^ (context is None), \
"Context must always be added or never be added."
assert len(configurations) == len(scores), \
Expand Down Expand Up @@ -113,7 +117,9 @@ def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,
"""
pass # pylint: disable=unnecessary-pass # pragma: no cover

def suggest(self, context: Optional[pd.DataFrame] = None, defaults: bool = False) -> pd.DataFrame:
def suggest(
self, context: Optional[pd.DataFrame] = None, defaults: bool = False
) -> Tuple[pd.DataFrame, Optional[pd.DataFrame]]:
"""
Wrapper method, which employs the space adapter (if any), after suggesting a new configuration.

Expand All @@ -129,13 +135,26 @@ def suggest(self, context: Optional[pd.DataFrame] = None, defaults: bool = False
-------
configuration : pd.DataFrame
Pandas dataframe with a single row. Column names are the parameter names.

context : pd.DataFrame
Pandas dataframe with a single row containing the context.
Column names are the budget, seed, and instance of the evaluation, if valid.
"""
if defaults:
configuration = config_to_dataframe(self.parameter_space.get_default_configuration())
self.delayed_config, self.delayed_context = self._suggest(context)

configuration: pd.DataFrame = config_to_dataframe(
self.parameter_space.get_default_configuration()
)
context = self.delayed_context
if self.space_adapter is not None:
configuration = self.space_adapter.inverse_transform(configuration)
else:
configuration = self._suggest(context)
if self.delayed_config is None:
configuration, context = self._suggest(context)
else:
configuration, context = self.delayed_config, self.delayed_context
self.delayed_config, self.delayed_context = None, None
assert len(configuration) == 1, \
"Suggest must return a single configuration."
assert set(configuration.columns).issubset(set(self.optimizer_parameter_space)), \
Expand All @@ -144,10 +163,12 @@ def suggest(self, context: Optional[pd.DataFrame] = None, defaults: bool = False
configuration = self._space_adapter.transform(configuration)
assert set(configuration.columns).issubset(set(self.parameter_space)), \
"Space adapter produced a configuration that does not match the expected parameter space."
return configuration
return configuration, context

@abstractmethod
def _suggest(self, context: Optional[pd.DataFrame] = None) -> pd.DataFrame:
def _suggest(
self, context: Optional[pd.DataFrame] = None
) -> Tuple[pd.DataFrame, Optional[pd.DataFrame]]:
"""Suggests a new configuration.

Parameters
Expand Down
12 changes: 9 additions & 3 deletions mlos_core/mlos_core/optimizers/random_optimizer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
Contains the RandomOptimizer class.
"""

from typing import Optional
from typing import Optional, Tuple
from warnings import warn

import pandas as pd
Expand Down Expand Up @@ -45,7 +45,9 @@ def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,
warn(f"Not Implemented: Ignoring context {list(context.columns)}", UserWarning)
# should we pop them from self.pending_observations?

def _suggest(self, context: Optional[pd.DataFrame] = None) -> pd.DataFrame:
def _suggest(
self, context: Optional[pd.DataFrame] = None
) -> Tuple[pd.DataFrame, Optional[pd.DataFrame]]:
"""Suggests a new configuration.

Sampled at random using ConfigSpace.
Expand All @@ -59,11 +61,15 @@ def _suggest(self, context: Optional[pd.DataFrame] = None) -> pd.DataFrame:
-------
configuration : pd.DataFrame
Pandas dataframe with a single row. Column names are the parameter names.

context : pd.DataFrame
Pandas dataframe with a single row containing the context.
Column names are the budget, seed, and instance of the evaluation, if valid.
"""
if context is not None:
# not sure how that works here?
warn(f"Not Implemented: Ignoring context {list(context.columns)}", UserWarning)
return pd.DataFrame(dict(self.optimizer_parameter_space.sample_configuration()), index=[0])
return pd.DataFrame(dict(self.optimizer_parameter_space.sample_configuration()), index=[0]), None
Copy link
Contributor

Choose a reason for hiding this comment

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

Why should this return None for context?

Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't returning the context that was passed in make more sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have interpreted the context that is passed in to the suggest to be different than the context that is returned. I am not exactly sure what the context in is supposed to mean

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's what I expect:

  • context is composed of some set of metrics describing the execution environment (e.g., vm ram, vm vcore count, workload descriptor, etc.)
  • upon registering the optimizer is able to build a model for how different config+context pairs maps to different output metrics
  • when requesting a new suggestion, possible for a different execution environment (e.g., larger vm size), described in the context, the optimizer should be able to use all of that information to provide a new config suggestion for that new context

Now, it's possible that the optimizer stinks at that part initially because it doesn't know how to compute suggestions for unseen contexts (future research work probably).
One option could be that we return a variable length (user specified) list of items, each stemming from different context of "known" items (ranked by higher confidence), instead of for a the new context and then the caller needs to sift through and decide what to do, or else we return a random config (explore) + the provided context and they could start trying them to help fill in that knowledge.
I think the exact specifics of that part is to be determined.

But high level, if someone asks for a suggestion for a given context, I think we should return something within context, not something for anywhere in the space.

Make sense?

Copy link
Contributor Author

@jsfreischuetz jsfreischuetz Jun 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

context is composed of some set of metrics describing the execution environment (e.g., vm ram, vm vcore count, workload descriptor, etc.)

As far as I am aware this is not supported by any of the optimizers when asking for a suggestion. SMAC supports the idea of these values being defined on optimizer initialization, but which context to evaluate is determined by the optimizer, and there is still not support for requiring a specific context.

But high level, if someone asks for a suggestion for a given context, I think we should return something within context, not something for anywhere in the space.

I think there is a problem with this. There are two ways to solve for this:

  1. If we simply request configurations until we find a context that matches, we now have a long list of pending configurations that should be evaluated according to the optimizer. If you ignore them, the optimizer will not be evaluating in this region as it believes that they are still pending. If you process them late, they are stale.
  2. Alternatively, we can force the context to match, regardless of what the optimizer returns, but this is not actually sampling efficiently according to the optimizer.

I think realistically, we should remove the argument from suggest, but I tried to keep the changes as minimal as possible. Someone else before me also left a comment suggesting this.

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed offline about the discrepency in terminology.

SMAC's notion of "context" is different from that in MLOS, which @jsfreischuetz rightly pointed out may or may not be supported directly yet.
SMAC's is much more about passing internal optimizer "state".

For now, we decided to

  1. document the difference in the README.md above.
  2. rename the SMAC required "state" to either metadata or state


def register_pending(self, configurations: pd.DataFrame,
context: Optional[pd.DataFrame] = None) -> None:
Expand Down
5 changes: 1 addition & 4 deletions mlos_core/mlos_core/tests/optimizers/bayesian_optimizers_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,10 @@ def test_context_not_implemented_warning(configuration_space: CS.ConfigurationSp
optimization_targets=['score'],
**kwargs
)
suggestion = optimizer.suggest()
suggestion, _ = optimizer.suggest()
scores = pd.DataFrame({'score': [1]})
context = pd.DataFrame([["something"]])

with pytest.raises(UserWarning):
optimizer.register(suggestion, scores, context=context)

with pytest.raises(UserWarning):
optimizer.suggest(context=context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, so we can register context, but not suggest with it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's more about not sending the warning not being being thrown anymore

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand what the code in this test is doing. I'm asking why we aren't supporting suggestions with context yet when we're adding support for registering with context. It makes it seem like a blackhole of information or only partial support somehow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does context for a suggestion mean?

Copy link
Contributor

Choose a reason for hiding this comment

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

We discussed offline and is documented in the suggestion in the README.md above now.


Expand Down
6 changes: 2 additions & 4 deletions mlos_core/mlos_core/tests/optimizers/optimizer_multiobj_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
optimizer.get_observations()

for _ in range(max_iterations):
suggestion = optimizer.suggest()
suggestion, context = optimizer.suggest()
assert isinstance(suggestion, pd.DataFrame)
assert set(suggestion.columns) == {'x', 'y'}
# Check suggestion values are the expected dtype
Expand All @@ -76,12 +76,11 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
observation = objective(suggestion)
assert isinstance(observation, pd.DataFrame)
assert set(observation.columns) == {'score', 'other_score'}
optimizer.register(suggestion, observation)
optimizer.register(suggestion, observation, context)

(best_config, best_score, best_context) = optimizer.get_best_observations()
assert isinstance(best_config, pd.DataFrame)
assert isinstance(best_score, pd.DataFrame)
assert best_context is None
assert set(best_config.columns) == {'x', 'y'}
assert set(best_score.columns) == {'score', 'other_score'}
assert best_config.shape == (1, 2)
Expand All @@ -90,7 +89,6 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
(all_configs, all_scores, all_contexts) = optimizer.get_observations()
assert isinstance(all_configs, pd.DataFrame)
assert isinstance(all_scores, pd.DataFrame)
assert all_contexts is None
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. Can we assert that the contexts are something useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same as above, unless we split out the SMAC tests

Copy link
Contributor

Choose a reason for hiding this comment

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

Don't need to split them out, just add a branch in the test for them.
I want to make sure we're testing expected functionality in both directions.
It also makes it more obvious what needs to be done when we extend it elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the associated changes

assert set(all_configs.columns) == {'x', 'y'}
assert set(all_scores.columns) == {'score', 'other_score'}
assert all_configs.shape == (max_iterations, 2)
Expand Down
26 changes: 10 additions & 16 deletions mlos_core/mlos_core/tests/optimizers/optimizer_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def test_create_optimizer_and_suggest(configuration_space: CS.ConfigurationSpace

assert optimizer.parameter_space is not None

suggestion = optimizer.suggest()
suggestion, context = optimizer.suggest()
assert suggestion is not None

myrepr = repr(optimizer)
Expand Down Expand Up @@ -94,7 +94,7 @@ def objective(x: pd.Series) -> pd.DataFrame:
optimizer.get_observations()

for _ in range(max_iterations):
suggestion = optimizer.suggest()
suggestion, context = optimizer.suggest()
assert isinstance(suggestion, pd.DataFrame)
assert set(suggestion.columns) == {'x', 'y', 'z'}
# check that suggestion is in the space
Expand All @@ -103,12 +103,11 @@ def objective(x: pd.Series) -> pd.DataFrame:
configuration.is_valid_configuration()
observation = objective(suggestion['x'])
assert isinstance(observation, pd.DataFrame)
optimizer.register(suggestion, observation)
optimizer.register(suggestion, observation, context)

(best_config, best_score, best_context) = optimizer.get_best_observations()
assert isinstance(best_config, pd.DataFrame)
assert isinstance(best_score, pd.DataFrame)
assert best_context is None
assert set(best_config.columns) == {'x', 'y', 'z'}
assert set(best_score.columns) == {'score'}
assert best_config.shape == (1, 3)
Expand All @@ -118,7 +117,6 @@ def objective(x: pd.Series) -> pd.DataFrame:
(all_configs, all_scores, all_contexts) = optimizer.get_observations()
assert isinstance(all_configs, pd.DataFrame)
assert isinstance(all_scores, pd.DataFrame)
assert all_contexts is None
assert set(all_configs.columns) == {'x', 'y', 'z'}
assert set(all_scores.columns) == {'score'}
assert all_configs.shape == (20, 3)
Expand Down Expand Up @@ -176,7 +174,7 @@ def test_create_optimizer_with_factory_method(configuration_space: CS.Configurat

assert optimizer.parameter_space is not None

suggestion = optimizer.suggest()
suggestion, _ = optimizer.suggest()
assert suggestion is not None

if optimizer_type is not None:
Expand Down Expand Up @@ -268,16 +266,16 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
_LOG.debug("Optimizer is done with random init.")

# loop for optimizer
suggestion = optimizer.suggest()
suggestion, context = optimizer.suggest()
observation = objective(suggestion)
optimizer.register(suggestion, observation)
optimizer.register(suggestion, observation, context)

# loop for llamatune-optimizer
suggestion = llamatune_optimizer.suggest()
suggestion, context = llamatune_optimizer.suggest()
_x, _y = suggestion['x'].iloc[0], suggestion['y'].iloc[0]
assert _x == pytest.approx(_y, rel=1e-3) or _x + _y == pytest.approx(3., rel=1e-3) # optimizer explores 1-dimensional space
observation = objective(suggestion)
llamatune_optimizer.register(suggestion, observation)
llamatune_optimizer.register(suggestion, observation, context)

# Retrieve best observations
best_observation = optimizer.get_best_observations()
Expand All @@ -286,7 +284,6 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
for (best_config, best_score, best_context) in (best_observation, llamatune_best_observation):
assert isinstance(best_config, pd.DataFrame)
assert isinstance(best_score, pd.DataFrame)
assert best_context is None
assert set(best_config.columns) == {'x', 'y'}
assert set(best_score.columns) == {'score'}

Expand All @@ -302,7 +299,6 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
optimizer.get_observations(), llamatune_optimizer.get_observations()):
assert isinstance(all_configs, pd.DataFrame)
assert isinstance(all_scores, pd.DataFrame)
assert all_contexts is None
assert set(all_configs.columns) == {'x', 'y'}
assert set(all_scores.columns) == {'score'}
assert len(all_configs) == num_iters
Expand Down Expand Up @@ -375,7 +371,7 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
optimizer.get_observations()

for _ in range(max_iterations):
suggestion = optimizer.suggest()
suggestion, context = optimizer.suggest()
assert isinstance(suggestion, pd.DataFrame)
assert (suggestion.columns == ['x', 'y']).all()
# Check suggestion values are the expected dtype
Expand All @@ -388,14 +384,12 @@ def objective(point: pd.DataFrame) -> pd.DataFrame:
# Test registering the suggested configuration with a score.
observation = objective(suggestion)
assert isinstance(observation, pd.DataFrame)
optimizer.register(suggestion, observation)
optimizer.register(suggestion, observation, context)

(best_config, best_score, best_context) = optimizer.get_best_observations()
assert isinstance(best_config, pd.DataFrame)
assert isinstance(best_score, pd.DataFrame)
assert best_context is None

(all_configs, all_scores, all_contexts) = optimizer.get_observations()
assert isinstance(all_configs, pd.DataFrame)
assert isinstance(all_scores, pd.DataFrame)
assert all_contexts is None
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we assert that it is something expected now instead of just removing the check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, because only SMAC uses context, where there other optimizers do not

Copy link
Contributor

Choose a reason for hiding this comment

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

Then check for that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the associated changes

489 changes: 225 additions & 264 deletions mlos_core/notebooks/BayesianOptimization.ipynb
View file
Open in desktop

Large diffs are not rendered by default.