Enable Optimizer state and Multi-Fidelity passthrough in SMAC by jsfreischuetz · Pull Request #751 · microsoft/MLOS

Johannes Freischuetz (jsfreischuetz) · 2024-05-22T20:31:17Z

Enables changing the defaults in the SMAC optimizer to enable state passing (what SMAC internally calls context, but that doesn't match MLOS's notion of context) and multi-fidelity tuning.
This requires changes across the project to support the concept of state, as well as updating a few test cases.

This PR replaces the previous one (#741)

Brian Kroth (bpkroth) · 2024-06-03T22:25:33Z

Johannes Freischuetz (@jsfreischuetz) I just merged #738 from Sergiy Matusevych (@motus).
Can you please update your branch to resolve conflicts?
Thanks!

Brian Kroth (bpkroth) · 2024-06-03T22:27:57Z

    (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


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

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

Then check for that too.

I made the associated changes

Brian Kroth (bpkroth) · 2024-06-03T22:28:34Z

    (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


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

Same as above, unless we split out the SMAC tests

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.

I made the associated changes

Brian Kroth (bpkroth) · 2024-06-03T22:29:05Z

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


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

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

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.

What does context for a suggestion mean?

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

Brian Kroth (bpkroth) · 2024-06-03T22:29:37Z

            # 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


Why should this return None for context?

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

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

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?

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:

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.

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.

We discussed offline about the discrepency in terminology.

SMAC's notion of "context" is different from that in MLOS, which Johannes Freischuetz (@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

document the difference in the README.md above.

rename the SMAC required "state" to either metadata or state

Co-authored-by: Brian Kroth <[email protected]>

Brian Kroth (bpkroth)

Please restore context and add metadata as new optional parameters.

Brian Kroth (bpkroth) · 2024-06-04T18:13:57Z

-
+        self.trial_info_df: pd.DataFrame = pd.DataFrame(
+            columns=["Configuration", "Context", "TrialInfo", "TrialValue"]
+        )


TODO: look into abstracting this out so multiple backend optimizers can reuse it (e.g., FLAML, LlamaTune)

Brian Kroth (bpkroth) · 2024-06-04T18:18:02Z


    @staticmethod
-    def _dummy_target_func(config: ConfigSpace.Configuration, seed: int = 0) -> None:
+    def _filter_kwargs(function: Callable, **kwargs: Any) -> Dict[str, Any]:


Move to util.py?

Brian Kroth (bpkroth) · 2024-06-06T17:15:19Z


    def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,
-                  context: Optional[pd.DataFrame] = None) -> None:
+                  metadata: Optional[pd.DataFrame] = None) -> None:


Please restore context.
The idea was to add metadata in addition to it, not replace it.

(requires associated changes throughout)

Brian Kroth (bpkroth) · 2024-06-06T17:17:26Z

            # 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


We discussed offline about the discrepency in terminology.

SMAC's notion of "context" is different from that in MLOS, which Johannes Freischuetz (@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

document the difference in the README.md above.

rename the SMAC required "state" to either metadata or state

Brian Kroth (bpkroth) · 2024-06-06T17:17:43Z

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


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

…zer.py Co-authored-by: Brian Kroth <[email protected]>

Co-authored-by: Brian Kroth <[email protected]>

This also involves modifying the test cases that interact with this interfaces

Brian Kroth (bpkroth) · 2024-06-12T16:50:41Z

        # Do some input validation.
-        assert set(scores.columns) == set(self._optimization_targets), \
-            "Mismatched optimization targets."
+        if type(self._optimization_targets) is str:


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:

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

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

OK, but separate PR for that one please

Co-authored-by: Brian Kroth <[email protected]>

Brian Kroth (bpkroth) · 2024-06-12T16:53:18Z

            assert configurations.shape[1] == len(self.optimizer_parameter_space.values()), \
                "Mismatched configuration shape after inverse transform."
-        return self._register(configurations, scores, context)
+        return self._register(configurations, scores, metadata, context)


Brian Kroth (bpkroth) · 2024-06-12T16:54:04Z

+        return self._register(configurations, scores, metadata, context)

    @abstractmethod
    def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,


Suggested change

def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,

def _register(self, *, configurations: pd.DataFrame, scores: pd.DataFrame,

Can force the args to be named to help avoid param ordering mistakes.

Same for the elsewhere (e.g., public methods and suggest), though this might be a larger API change that needs its own PR first in prepration for this one since callers will also be affected.

Agree. Let's fix _register now and update the public register in the next PR

Brian Kroth (bpkroth) · 2024-06-12T16:58:23Z

+            configuration: pd.DataFrame = config_to_dataframe(
+                self.parameter_space.get_default_configuration()
+            )
+            metadata = self.delayed_metadata


nit: when creating PRs - try to keep your changes smaller. It's easier to review and debug.
If the order of this one didn't really matter you could have left the first line alone and only added the two new ones

Co-authored-by: Brian Kroth <[email protected]>

Sergiy Matusevych (motus)

Left a few comments. Let's have a meeting and discuss the changes. I am a bit confused about the purpose of the metadata and how it is different from the context

Sergiy Matusevych (motus) · 2024-06-18T23:05:59Z


        self._space_adapter: Optional[BaseSpaceAdapter] = space_adapter
-        self._observations: List[Tuple[pd.DataFrame, pd.DataFrame, Optional[pd.DataFrame]]] = []
+        self._observations: List[Tuple[pd.DataFrame, pd.DataFrame, Optional[pd.DataFrame], Optional[pd.DataFrame]]] = []


this gets a bit unwieldy. Should we have a named tuple for _pending_observations, e.g.,

class PendingObservation(NamedTuple): """A named tuple representing a pending observation.""" configurations: pd.DataFrame context: Optional[pd.DataFrame] meta: Optional[pd.DataFrame]

and do the same for _observations? (not sure we can inherit NamedTuples - most likely, we can't)

or, maybe, have _observed_configs, _observed_scores, _observed_contexts etc. and concatenate the dataframes instead of having a list of dataframes. I am pretty sure the schemas are the same from one _register call to the next

Sergiy Matusevych (motus) · 2024-06-18T23:11:57Z

+        self.delayed_config: Optional[pd.DataFrame] = None
+        self.delayed_metadata: Optional[pd.DataFrame] = None


Suggested change

self.delayed_config: Optional[pd.DataFrame] = None

self.delayed_metadata: Optional[pd.DataFrame] = None

self._delayed_config: Optional[pd.DataFrame] = None

self._delayed_metadata: Optional[pd.DataFrame] = None

probably should be private, right?

Sergiy Matusevych (motus) · 2024-06-18T23:14:50Z

+        return self._register(configurations, scores, metadata, context)

    @abstractmethod
    def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,


Agree. Let's fix _register now and update the public register in the next PR

Sergiy Matusevych (motus) · 2024-06-18T23:19:18Z

        -------
        observations : Tuple[pd.DataFrame, pd.DataFrame, Optional[pd.DataFrame]]
-            A triplet of (config, score, context) DataFrames of observations.
+            A triplet of (config, score, metadata) DataFrames of observations.


Suggested change

A triplet of (config, score, metadata) DataFrames of observations.

A 4-tuple of (config, score, context, metadata) DataFrames of observations.

(or, better yet, a NamedTuple)

Sergiy Matusevych (motus) · 2024-06-18T23:24:08Z

-        configs = pd.concat([config for config, _, _ in self._observations]).reset_index(drop=True)
-        scores = pd.concat([score for _, score, _ in self._observations]).reset_index(drop=True)
+        configs = pd.concat([config for config, _, _, _ in self._observations]).reset_index(drop=True)
+        scores = pd.concat([score for _, score, _, _ in self._observations]).reset_index(drop=True)


Yeah, let's have a List[Observation] NamedTuples - or, better yet, concatenate the dataframes right there in _register and forget about List and NamedTuple

Sergiy Matusevych (motus) · 2024-06-18T23:40:29Z

It looks like we don't use this module anywhere - maybe remove it from this PR?

Sergiy Matusevych (motus) · 2024-06-18T23:41:38Z

    assert best_score.shape == (1, 2)

-    (all_configs, all_scores, all_contexts) = optimizer.get_observations()
+    (all_configs, all_scores, all_metadata, best_context) = optimizer.get_observations()


why best_context?? shouldn't it be all_contexts instead?

Sergiy Matusevych (motus) · 2024-06-18T23:42:52Z

+        assert isinstance(all_metadata, pd.DataFrame) or all_metadata is None
+    else:
+        assert all_metadata is None
+    assert best_context is None


Suggested change

assert best_context is None

assert all_contexts is None

?

Sergiy Matusevych (motus) · 2024-06-18T23:43:46Z

    assert optimizer.parameter_space is not None

-    suggestion = optimizer.suggest()
+    suggestion, _ = optimizer.suggest()


here and in other places:

Suggested change

suggestion, _ = optimizer.suggest()

suggestion, _metadata = optimizer.suggest()

it's better to spell out things

Sergiy Matusevych (motus) · 2024-06-18T23:47:18Z

-    (best_config, best_score, _context) = best_observation
-    (llamatune_best_config, llamatune_best_score, _context) = llamatune_best_observation
+    (best_config, best_score, _, _) = best_observation
+    (llamatune_best_config, llamatune_best_score, _metadata) = llamatune_best_observation


I am confused. shouldn't it be

Suggested change

(llamatune_best_config, llamatune_best_score, _metadata) = llamatune_best_observation

(llamatune_best_config, llamatune_best_score, _context, _metadata) = llamatune_best_observation

?
this should fail in tests

Brian Kroth (bpkroth) · 2024-06-26T18:07:21Z

Left a few comments. Let's have a meeting and discuss the changes. I am a bit confused about the purpose of the metadata and how it is different from the context

It should be in the README I asked for and also some internal chats we had, but essentially, metadata is for internal optimizer saved state (e.g., something SMAC specific) and context is more of an index into a subspace of a larger optimization problem (e.g., optimize for VMs with 8GB RAM & 4 vCores and a transactional workload by reusing data from VMs with 4GB RAM and 2 vCores). Internally we may wrap the later in multiple SMAC invocations for instance.

This is a simple PR that makes all arguments explicit for optimizer-related function calls in preparation to add additional arguments in #751 and make it easier to review. --------- Co-authored-by: Brian Kroth <[email protected]> Co-authored-by: Brian Kroth <[email protected]>

Adds metadata to respond from suggest, and be passable into register. This is in support of adding multi-fidelity support (#751) --------- Co-authored-by: Brian Kroth <[email protected]> Co-authored-by: Brian Kroth <[email protected]>

minimal implementation of mutli-fidelity

7f8a43b

Johannes Freischuetz (jsfreischuetz) mentioned this pull request May 22, 2024

Enable context for SMAC Optimizer #741

Closed

Johannes Freischuetz (jsfreischuetz) changed the title ~~minimal implementation of mutli-fidelity~~ Enable Context and Multi-Fidelity passthrough in SMAC May 22, 2024

Brian Kroth (bpkroth) reviewed May 23, 2024

View reviewed changes

Comment thread .vscode/settings.json Outdated

Johannes Freischuetz (jsfreischuetz) added 2 commits May 23, 2024 23:00

revert changes

8afb5f0

revert

08575af

Johannes Freischuetz (jsfreischuetz) marked this pull request as ready for review May 24, 2024 20:48

Johannes Freischuetz (jsfreischuetz) requested a review from a team as a code owner May 24, 2024 20:48

Johannes Freischuetz (jsfreischuetz) added 2 commits June 1, 2024 22:55

fix minor bug with logging

fcfca53

undo formatting

838c1db

Brian Kroth (bpkroth) reviewed Jun 3, 2024

View reviewed changes

Comment thread mlos_core/mlos_core/optimizers/optimizer.py Outdated

Johannes Freischuetz (jsfreischuetz) and others added 4 commits June 3, 2024 17:58

Update mlos_core/mlos_core/optimizers/optimizer.py

7533b4e

Co-authored-by: Brian Kroth <[email protected]>

merge

3904020

Merge branch 'microsoft-main' into multifidleity

4ffff6c

merge

b7de120

Brian Kroth (bpkroth) reviewed Jun 4, 2024

View reviewed changes

Comment thread .vscode/settings.json

Johannes Freischuetz (jsfreischuetz) added 5 commits June 3, 2024 22:36

add checks back to optimizer

7278994

add checks back

c79294a

add checks back

048269c

update name of context to metadata, and add readme

019192a

update tests to also use correct terminology

88d63c1

Brian Kroth (bpkroth) changed the title ~~Enable Context and Multi-Fidelity passthrough in SMAC~~ Enable Optimizer state and Multi-Fidelity passthrough in SMAC Jun 6, 2024

Brian Kroth (bpkroth) suggested changes Jun 6, 2024

View reviewed changes

Johannes Freischuetz (jsfreischuetz) and others added 2 commits June 6, 2024 14:08

Update mlos_core/mlos_core/optimizers/bayesian_optimizers/smac_optimi…

4e36f28

…zer.py Co-authored-by: Brian Kroth <[email protected]>

Update mlos_core/mlos_core/optimizers/README.md

3326ac9

Co-authored-by: Brian Kroth <[email protected]>

Johannes Freischuetz (jsfreischuetz) and others added 3 commits June 6, 2024 14:11

Update mlos_core/mlos_core/optimizers/README.md

2399d3e

Co-authored-by: Brian Kroth <[email protected]>

Add context back to the register interface

1f210b5

This also involves modifying the test cases that interact with this interfaces

Merge branch 'main' into multifidleity

87a5af9

Johannes Freischuetz (jsfreischuetz) requested a review from Brian Kroth (bpkroth) June 7, 2024 22:55

Apply suggestions from code review

48af70f

Brian Kroth (bpkroth) reviewed Jun 12, 2024

View reviewed changes

Comment thread mlos_core/mlos_core/optimizers/optimizer.py Outdated

Brian Kroth (bpkroth) reviewed Jun 12, 2024

View reviewed changes

Comment thread mlos_core/mlos_core/optimizers/optimizer.py Outdated

Brian Kroth (bpkroth) reviewed Jun 12, 2024

View reviewed changes

Comment thread mlos_core/mlos_core/optimizers/optimizer.py Outdated

Sergiy Matusevych (motus) and others added 3 commits June 12, 2024 10:08

Merge branch 'main' into multifidleity

cd8deff

Update mlos_core/mlos_core/optimizers/optimizer.py

271a79b

Co-authored-by: Brian Kroth <[email protected]>

Update mlos_core/mlos_core/optimizers/optimizer.py

98c7398

Co-authored-by: Brian Kroth <[email protected]>

Brian Kroth (bpkroth) suggested changes Jun 12, 2024

View reviewed changes

Johannes Freischuetz (jsfreischuetz) and others added 2 commits June 12, 2024 17:04

Update mlos_core/mlos_core/optimizers/optimizer.py

9726410

Co-authored-by: Brian Kroth <[email protected]>

Update mlos_core/mlos_core/optimizers/optimizer.py

bf4602b

Co-authored-by: Brian Kroth <[email protected]>

Brian Kroth (bpkroth) reviewed Jun 13, 2024

View reviewed changes

Comment thread mlos_core/mlos_core/optimizers/flaml_optimizer.py Outdated

fix comments for python

8d2a894

Johannes Freischuetz (jsfreischuetz) force-pushed the multifidleity branch from c29405c to 8d2a894 Compare June 14, 2024 05:10

Sergiy Matusevych (motus) reviewed Jun 18, 2024

View reviewed changes

Johannes Freischuetz (jsfreischuetz) mentioned this pull request Jun 28, 2024

Require explicit arguments for mlos_core optimizers #760

Merged

Johannes Freischuetz (jsfreischuetz) mentioned this pull request Jul 1, 2024

Add Metadata to optimizers #770

Merged

Johannes Freischuetz (jsfreischuetz) mentioned this pull request Jul 2, 2024

Implement Metadata for SMAC enabling Multi-Fidelity #771

Open

Johannes Freischuetz (jsfreischuetz) closed this Jul 2, 2024

Brian Kroth (bpkroth) mentioned this pull request Jul 23, 2024

dataclasses for mlos_core Optimizer return values #810

Closed

Johannes Freischuetz (jsfreischuetz) mentioned this pull request Jul 23, 2024

Introduce Named Data Classes for mlos_core #811

Closed

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

	def _register(self, configurations: pd.DataFrame, scores: pd.DataFrame,
	def _register(self, *, configurations: pd.DataFrame, scores: pd.DataFrame,

		self.delayed_config: Optional[pd.DataFrame] = None
		self.delayed_metadata: Optional[pd.DataFrame] = None

	A triplet of (config, score, metadata) DataFrames of observations.
	A 4-tuple of (config, score, context, metadata) DataFrames of observations.

	suggestion, _ = optimizer.suggest()
	suggestion, _metadata = optimizer.suggest()

	(llamatune_best_config, llamatune_best_score, _metadata) = llamatune_best_observation
	(llamatune_best_config, llamatune_best_score, _context, _metadata) = llamatune_best_observation

Conversation

Johannes Freischuetz (jsfreischuetz) commented May 22, 2024 • edited by bpkroth Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Brian Kroth (bpkroth) commented Jun 3, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Johannes Freischuetz (jsfreischuetz) Jun 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Brian Kroth (bpkroth) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Johannes Freischuetz (jsfreischuetz) commented May 22, 2024 •

edited by bpkroth

Loading

Johannes Freischuetz (jsfreischuetz) Jun 4, 2024 •

edited

Loading