Allow get_config to return the same config more than once #487

mseeger · 2023-01-04T08:29:26Z

Issue #, if available: 415

Description of changes:
Introduces flag allow_duplicates to searchers, which allows to return the same config more than once. Also contains a new test on searchers, whether they properly implement allow_duplicates=False (the default).

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

mseeger · 2023-01-11T14:39:33Z

syne_tune/optimizer/schedulers/searchers/bore/bore.py

@@ -122,6 +127,11 @@ def __init__(

        self._hp_ranges = make_hyperparameter_ranges(self.config_space)
        self._excl_list = ExclusionList.empty_list(self._hp_ranges)
+        self._allow_duplicates = allow_duplicates
+        # Maps ``trial_id`` to configuration. This is used to blacklist


This is a technicality:

If not allow_duplicates, then _excl_list collects all configs suggested, so none is suggested 2x

If allow_duplicates, then _excl_list only collects configs for which the trial failed. In this case, we need to know the config for a trial, because evaluation_failed only gets the trial_id.

The same code is used in other places, same comment applies.

mseeger · 2023-01-11T14:40:38Z

syne_tune/optimizer/schedulers/searchers/bore/bore.py

+    def evaluation_failed(self, trial_id: str):
+        if self._allow_duplicates and trial_id in self._config_for_trial_id:
+            # Blacklist this configuration
+            self._excl_list.add(self._config_for_trial_id[trial_id])


Only get trial_id here, but need to know the config. This is why _config_for_trial is needed, only if allow_duplicates.

wistuba · 2023-01-11T21:07:35Z

syne_tune/optimizer/schedulers/searchers/random_grid_searcher.py

+    def register_pending(
+        self,
+        trial_id: str,
+        config: Optional[dict] = None,
+        milestone: Optional[int] = None,
+    ):
+        if self._allow_duplicates and trial_id not in self._config_for_trial_id:
+            if config is not None:
+                self._config_for_trial_id[trial_id] = config
+            else:
+                logger.warning(
+                    f"register_pending called for trial_id {trial_id} without passing config"
+                )
+
+    def evaluation_failed(self, trial_id: str):
+        if self._allow_duplicates and trial_id in self._config_for_trial_id:
+            # Blacklist this configuration
+            self._excl_list.add(self._config_for_trial_id[trial_id])


I've seen these functions now a couple of times. Is it possible to move it up in the class hierarchy or use a utils function instead?

Yes, doing that

wistuba · 2023-01-11T21:10:57Z

tst/schedulers/test_searchers.py

+        (KDE, False),
+    ],
+)
+def test_allow_duplicates_or_not(scheduler_cls, multifid):


seems natural to move allow_duplicates here

What do you mean?

add another parametrize line with allow_duplicates before the function

mseeger · 2023-01-17T15:49:58Z

Hi @wistuba , I am still working on this. I'd like to extend the unit test to the case where all trials fail, which should filter them out even if allow_duplicates == True. But this does not work right now.

mseeger · 2023-01-20T11:09:09Z

OK, I am done with this now

wistuba

The multi-inheritance is not a good choice here. What you could do instead is make it part of the base searcher. Searchers who don't support it can simply hard code the choice of allow_duplicates to False.

wistuba · 2023-01-20T11:42:21Z

syne_tune/optimizer/schedulers/synchronous/hyperband.py

@@ -290,6 +290,9 @@ def _suggest(self, trial_id: int) -> Optional[TrialSuggestion]:
            config = self.searcher.get_config(trial_id=str(trial_id))
            if config is not None:
                config = cast_config_values(config, self.config_space)
+                self.searcher.register_pending(


are these changes related to the feature?

Yes, this is something that was missing and came to light because of the new test

wistuba · 2023-01-20T11:44:31Z

syne_tune/optimizer/schedulers/searchers/searcher.py

@@ -450,3 +451,72 @@ def _restore_from_state(self, state: dict):

    def set_random_state(self, random_state: np.random.RandomState):
        self.random_state = random_state
+
+
+class FilterDuplicatesMixin:


the mixin should not have functions overlapping with BaseSearcher

Yes, so this would basically be multiple inheritance then.

The problem with moving this into BaseSearcher:

BaseSearcher is really supposed to be just an API. David would rightly reject putting code in there

Some of the searchers do this allow_duplicates in a different way

Is multiple inheritance really so evil?

OK, I think what I can do, is I can integrate this code into SearcherWithRandomSeed. This way, the BaseSearcher remains clean.

Or better: make it a subclass of that one, this is best

…duplicate filtering across all schedulers

wistuba

There are few lines of code that might be suited in the newly added class.

wistuba · 2023-01-23T08:35:15Z

syne_tune/optimizer/schedulers/searchers/bore/bore.py

+        if allow_duplicates is None:
+            allow_duplicates = False


deal with this in parent class

wistuba · 2023-01-23T08:36:32Z

syne_tune/optimizer/schedulers/searchers/bore/bore.py

+            if not self.allow_duplicates:
+                self._excl_list.add(config)  # Should not be suggested again


can't we delegate to parent class as well?

I'll try something

wistuba · 2023-01-23T08:39:55Z

syne_tune/optimizer/schedulers/searchers/kde/kde_searcher.py

+        if allow_duplicates is None:
+            allow_duplicates = False


mseeger requested a review from geoalgo January 4, 2023 08:29

mseeger mentioned this pull request Jan 4, 2023

Allow for independence between number of trials and number of combinations. #415

Closed

mseeger requested review from wesk and removed request for geoalgo January 8, 2023 21:04

mseeger force-pushed the allow_duplicates branch 4 times, most recently from 6bcad5d to 6c9c4d3 Compare January 11, 2023 14:00

mseeger commented Jan 11, 2023

View reviewed changes

mseeger requested review from wistuba and removed request for wesk January 11, 2023 14:44

wistuba reviewed Jan 11, 2023

View reviewed changes

mseeger force-pushed the allow_duplicates branch from bc54f77 to 90864c8 Compare January 17, 2023 15:50

mseeger force-pushed the allow_duplicates branch from 02445a3 to 4afa556 Compare January 20, 2023 11:11

wistuba requested changes Jan 20, 2023

View reviewed changes

mseeger force-pushed the allow_duplicates branch 3 times, most recently from 29a9bc1 to 1b6b396 Compare January 21, 2023 12:31

Allow get_config to return the same config more than once, and unify …

d77523c

…duplicate filtering across all schedulers

mseeger force-pushed the allow_duplicates branch from 1b6b396 to d77523c Compare January 21, 2023 12:54

mseeger mentioned this pull request Jan 21, 2023

Refactoring and renaming of BoTorchSearcher and BoTorch #507

Merged

wistuba reviewed Jan 23, 2023

View reviewed changes

Unified more code

a4c8cf3

wistuba self-requested a review January 23, 2023 09:11

wistuba previously approved these changes Jan 23, 2023

View reviewed changes

Lint fix

fb6ee6e

mseeger dismissed wistuba’s stale review via fb6ee6e January 23, 2023 09:12

wistuba approved these changes Jan 23, 2023

View reviewed changes

mseeger merged commit 894ad84 into main Jan 23, 2023

mseeger deleted the allow_duplicates branch January 23, 2023 09:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow get_config to return the same config more than once #487

Allow get_config to return the same config more than once #487

mseeger commented Jan 4, 2023

mseeger Jan 11, 2023

mseeger Jan 11, 2023

wistuba Jan 11, 2023

mseeger Jan 12, 2023

wistuba Jan 11, 2023

mseeger Jan 12, 2023

wistuba Jan 12, 2023

mseeger commented Jan 17, 2023

mseeger commented Jan 20, 2023

wistuba left a comment

wistuba Jan 20, 2023

mseeger Jan 20, 2023

wistuba Jan 20, 2023

mseeger Jan 20, 2023

mseeger Jan 20, 2023

mseeger Jan 20, 2023

wistuba left a comment

wistuba Jan 23, 2023

mseeger Jan 23, 2023

wistuba Jan 23, 2023

mseeger Jan 23, 2023

wistuba Jan 23, 2023

		if not self.allow_duplicates:
		self._excl_list.add(config) # Should not be suggested again

Allow get_config to return the same config more than once #487

Allow get_config to return the same config more than once #487

Conversation

mseeger commented Jan 4, 2023

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

mseeger commented Jan 17, 2023

mseeger commented Jan 20, 2023

wistuba 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

wistuba 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