From 55ae567f7a4c6213543d002b42270569454b5378 Mon Sep 17 00:00:00 2001 From: Kai Fricke Date: Fri, 18 Dec 2020 10:33:12 +0100 Subject: [PATCH] [tune] Fix and enable SigOpt tests (#12877) Co-authored-by: Richard Liaw --- .travis.yml | 1 - python/ray/tune/BUILD | 50 +++++++------- python/ray/tune/examples/sigopt_example.py | 18 +++-- .../sigopt_multi_objective_example.py | 21 ++++-- .../examples/sigopt_prior_beliefs_example.py | 20 ++++-- python/ray/tune/suggest/bayesopt.py | 8 ++- python/ray/tune/suggest/nevergrad.py | 2 +- python/ray/tune/suggest/sigopt.py | 26 ++++++-- python/ray/tune/suggest/suggestion.py | 6 ++ python/ray/tune/suggest/zoopt.py | 8 ++- python/ray/tune/tests/test_tune_restore.py | 65 +++++++++++++++---- 11 files changed, 158 insertions(+), 67 deletions(-) diff --git a/.travis.yml b/.travis.yml index d946fed22..a8ddb0d46 100644 --- a/.travis.yml +++ b/.travis.yml @@ -577,4 +577,3 @@ deploy: repo: ray-project/ray branch: master condition: $MULTIPLATFORM_JARS = 1 || $MAC_JARS = 1 || $LINUX_JARS = 1 - diff --git a/python/ray/tune/BUILD b/python/ray/tune/BUILD index 8b3439853..70fd218d5 100644 --- a/python/ray/tune/BUILD +++ b/python/ray/tune/BUILD @@ -254,7 +254,7 @@ py_test( size = "large", srcs = ["tests/test_tune_restore.py"], deps = [":tune_lib"], - tags = ["jenkins_only", "exclusive"], + tags = ["exclusive"], ) py_test( @@ -648,35 +648,35 @@ py_test( # ) # Needs SigOpt API key. -# py_test( -# name = "sigopt_example", -# size = "medium", -# srcs = ["examples/sigopt_example.py"], -# deps = [":tune_lib"], -# tags = ["exclusive", "example"], -# args = ["--smoke-test"] -# ) +py_test( + name = "sigopt_example", + size = "medium", + srcs = ["examples/sigopt_example.py"], + deps = [":tune_lib"], + tags = ["exclusive", "example"], + args = ["--smoke-test"] +) # Needs SigOpt API key. -# py_test( -# name = "sigopt_multi_objective_example", -# size = "medium", -# srcs = ["examples/sigopt_multi_objective_example.py"], -# deps = [":tune_lib"], s -# tags = ["exclusive", "example"], -# args = ["--smoke-test"] -# ) +py_test( + name = "sigopt_multi_objective_example", + size = "medium", + srcs = ["examples/sigopt_multi_objective_example.py"], + deps = [":tune_lib"], + tags = ["exclusive", "example"], + args = ["--smoke-test"] +) # Needs SigOpt API key. -# py_test( -# name = "sigopt_prior_beliefs_example", -# size = "medium", -# srcs = ["examples/sigopt_prior_beliefs_example.py"], -# deps = [":tune_lib"], -# tags = ["exclusive", "example"], -# args = ["--smoke-test"] -# ) +py_test( + name = "sigopt_prior_beliefs_example", + size = "medium", + srcs = ["examples/sigopt_prior_beliefs_example.py"], + deps = [":tune_lib"], + tags = ["exclusive", "example"], + args = ["--smoke-test"] +) py_test( name = "skopt_example", diff --git a/python/ray/tune/examples/sigopt_example.py b/python/ray/tune/examples/sigopt_example.py index dabe38aa1..d765404e9 100644 --- a/python/ray/tune/examples/sigopt_example.py +++ b/python/ray/tune/examples/sigopt_example.py @@ -2,6 +2,7 @@ It also checks that it is usable with a separate scheduler. """ +import sys import time from ray import tune @@ -29,14 +30,20 @@ if __name__ == "__main__": import argparse import os - assert "SIGOPT_KEY" in os.environ, \ - "SigOpt API key must be stored as environment variable at SIGOPT_KEY" - parser = argparse.ArgumentParser() parser.add_argument( "--smoke-test", action="store_true", help="Finish quickly for testing") args, _ = parser.parse_known_args() + if "SIGOPT_KEY" not in os.environ: + if args.smoke_test: + print("SigOpt API Key not found. Skipping smoke test.") + sys.exit(0) + else: + raise ValueError( + "SigOpt API Key not found. Please set the SIGOPT_KEY " + "environment variable.") + space = [ { "name": "width", @@ -67,7 +74,8 @@ if __name__ == "__main__": name="my_exp", search_alg=algo, scheduler=scheduler, - num_samples=10 if args.smoke_test else 1000, + num_samples=4 if args.smoke_test else 100, config={"steps": 10}) - print("Best hyperparameters found were: ", analysis.best_config) + print("Best hyperparameters found were: ", + analysis.get_best_config("mean_loss", "min")) diff --git a/python/ray/tune/examples/sigopt_multi_objective_example.py b/python/ray/tune/examples/sigopt_multi_objective_example.py index 1d34da8cc..b1ec64332 100644 --- a/python/ray/tune/examples/sigopt_multi_objective_example.py +++ b/python/ray/tune/examples/sigopt_multi_objective_example.py @@ -1,5 +1,5 @@ """Example using Sigopt's multi-objective functionality.""" - +import sys import time import numpy as np @@ -30,14 +30,20 @@ if __name__ == "__main__": import argparse import os - assert "SIGOPT_KEY" in os.environ, \ - "SigOpt API key must be stored as environment variable at SIGOPT_KEY" - parser = argparse.ArgumentParser() parser.add_argument( "--smoke-test", action="store_true", help="Finish quickly for testing") args, _ = parser.parse_known_args() + if "SIGOPT_KEY" not in os.environ: + if args.smoke_test: + print("SigOpt API Key not found. Skipping smoke test.") + sys.exit(0) + else: + raise ValueError( + "SigOpt API Key not found. Please set the SIGOPT_KEY " + "environment variable.") + space = [ { "name": "w1", @@ -52,7 +58,7 @@ if __name__ == "__main__": algo = SigOptSearch( space, name="SigOpt Example Multi Objective Experiment", - observation_budget=10 if args.smoke_test else 1000, + observation_budget=4 if args.smoke_test else 100, max_concurrent=1, metric=["average", "std", "sharpe"], mode=["max", "min", "obs"]) @@ -61,6 +67,7 @@ if __name__ == "__main__": easy_objective, name="my_exp", search_alg=algo, - num_samples=10 if args.smoke_test else 1000, + num_samples=4 if args.smoke_test else 100, config={"total_weight": 1}) - print("Best hyperparameters found were: ", analysis.best_config) + print("Best hyperparameters found were: ", + analysis.get_best_config("average", "min")) diff --git a/python/ray/tune/examples/sigopt_prior_beliefs_example.py b/python/ray/tune/examples/sigopt_prior_beliefs_example.py index 420a342a8..a624a6a8f 100644 --- a/python/ray/tune/examples/sigopt_prior_beliefs_example.py +++ b/python/ray/tune/examples/sigopt_prior_beliefs_example.py @@ -1,4 +1,5 @@ """"Example using Sigopt's support for prior beliefs.""" +import sys import numpy as np from ray import tune @@ -37,14 +38,21 @@ if __name__ == "__main__": import os from sigopt import Connection - assert "SIGOPT_KEY" in os.environ, \ - "SigOpt API key must be stored as environment variable at SIGOPT_KEY" - parser = argparse.ArgumentParser() parser.add_argument( "--smoke-test", action="store_true", help="Finish quickly for testing") args, _ = parser.parse_known_args() - samples = 10 if args.smoke_test else 1000 + + if "SIGOPT_KEY" not in os.environ: + if args.smoke_test: + print("SigOpt API Key not found. Skipping smoke test.") + sys.exit(0) + else: + raise ValueError( + "SigOpt API Key not found. Please set the SIGOPT_KEY " + "environment variable.") + + samples = 4 if args.smoke_test else 100 conn = Connection(client_token=os.environ["SIGOPT_KEY"]) experiment = conn.experiments().create( @@ -95,4 +103,6 @@ if __name__ == "__main__": search_alg=algo, num_samples=samples, config={}) - print("Best hyperparameters found were: ", analysis.best_config) + + print("Best hyperparameters found were: ", + analysis.get_best_config("average", "min")) diff --git a/python/ray/tune/suggest/bayesopt.py b/python/ray/tune/suggest/bayesopt.py index df8f94546..489e92709 100644 --- a/python/ray/tune/suggest/bayesopt.py +++ b/python/ray/tune/suggest/bayesopt.py @@ -169,9 +169,7 @@ class BayesOptSearch(Searcher): self.utility = byo.UtilityFunction(**utility_kwargs) - # Registering the provided analysis, if given - if analysis is not None: - self.register_analysis(analysis) + self._analysis = analysis if isinstance(space, dict) and space: resolved_vars, domain_vars, grid_vars = parse_spec_vars(space) @@ -200,6 +198,10 @@ class BayesOptSearch(Searcher): verbose=self._verbose, random_state=self._random_state) + # Registering the provided analysis, if given + if self._analysis is not None: + self.register_analysis(self._analysis) + def set_search_properties(self, metric: Optional[str], mode: Optional[str], config: Dict) -> bool: if self.optimizer: diff --git a/python/ray/tune/suggest/nevergrad.py b/python/ray/tune/suggest/nevergrad.py index 669114d9b..f5da80b00 100644 --- a/python/ray/tune/suggest/nevergrad.py +++ b/python/ray/tune/suggest/nevergrad.py @@ -148,7 +148,7 @@ class NevergradSearch(Searcher): space = self.convert_search_space(space) if isinstance(optimizer, Optimizer): - if space is not None or isinstance(space, list): + if space is not None and not isinstance(space, list): raise ValueError( "If you pass a configured optimizer to Nevergrad, either " "pass a list of parameter names or None as the `space` " diff --git a/python/ray/tune/suggest/sigopt.py b/python/ray/tune/suggest/sigopt.py index 415e8aa3a..8bdcaaf1d 100644 --- a/python/ray/tune/suggest/sigopt.py +++ b/python/ray/tune/suggest/sigopt.py @@ -136,6 +136,7 @@ class SigOptSearch(Searcher): project: Optional[str] = None, metric: Union[None, str, List[str]] = "episode_reward_mean", mode: Union[None, str, List[str]] = "max", + points_to_evaluate: Optional[List[Dict]] = None, **kwargs): assert (experiment_id is None) ^ (space is None), "space xor experiment_id must be set" @@ -182,17 +183,25 @@ class SigOptSearch(Searcher): else: self.experiment = self.conn.experiments(experiment_id).fetch() + self._points_to_evaluate = points_to_evaluate + super(SigOptSearch, self).__init__(metric=metric, mode=mode, **kwargs) def suggest(self, trial_id: str): if self._max_concurrent: if len(self._live_trial_mapping) >= self._max_concurrent: return None + + suggestion_kwargs = {} + if self._points_to_evaluate: + config = self._points_to_evaluate.pop(0) + suggestion_kwargs = {"assignments": config} + # Get new suggestion from SigOpt suggestion = self.conn.experiments( - self.experiment.id).suggestions().create() + self.experiment.id).suggestions().create(**suggestion_kwargs) - self._live_trial_mapping[trial_id] = suggestion + self._live_trial_mapping[trial_id] = suggestion.id return copy.deepcopy(suggestion.assignments) @@ -210,7 +219,7 @@ class SigOptSearch(Searcher): """ if result: payload = dict( - suggestion=self._live_trial_mapping[trial_id].id, + suggestion=self._live_trial_mapping[trial_id], values=self.serialize_result(result)) self.conn.experiments( self.experiment.id).observations().create(**payload) @@ -219,7 +228,7 @@ class SigOptSearch(Searcher): elif error: # Reports a failed Observation self.conn.experiments(self.experiment.id).observations().create( - failed=True, suggestion=self._live_trial_mapping[trial_id].id) + failed=True, suggestion=self._live_trial_mapping[trial_id]) del self._live_trial_mapping[trial_id] @staticmethod @@ -254,12 +263,15 @@ class SigOptSearch(Searcher): return values def save(self, checkpoint_path: str): - trials_object = (self.conn, self.experiment) + trials_object = (self.experiment.id, self._live_trial_mapping, + self._points_to_evaluate) with open(checkpoint_path, "wb") as outputFile: pickle.dump(trials_object, outputFile) def restore(self, checkpoint_path: str): with open(checkpoint_path, "rb") as inputFile: trials_object = pickle.load(inputFile) - self.conn = trials_object[0] - self.experiment = trials_object[1] + experiment_id, self._live_trial_mapping, self._points_to_evaluate = \ + trials_object + + self.experiment = self.conn.experiments(experiment_id).fetch() diff --git a/python/ray/tune/suggest/suggestion.py b/python/ray/tune/suggest/suggestion.py index 99a6001d1..3612576d4 100644 --- a/python/ray/tune/suggest/suggestion.py +++ b/python/ray/tune/suggest/suggestion.py @@ -391,6 +391,12 @@ class ConcurrencyLimiter(Searcher): def set_state(self, state: Dict): self.__dict__.update(state) + def save(self, checkpoint_path: str): + self.searcher.save(checkpoint_path) + + def restore(self, checkpoint_path: str): + self.searcher.restore(checkpoint_path) + def on_pause(self, trial_id: str): self.searcher.on_pause(trial_id) diff --git a/python/ray/tune/suggest/zoopt.py b/python/ray/tune/suggest/zoopt.py index f9e3d04bb..c0c0ddb18 100644 --- a/python/ray/tune/suggest/zoopt.py +++ b/python/ray/tune/suggest/zoopt.py @@ -138,6 +138,7 @@ class ZOOptSearch(Searcher): metric: Optional[str] = None, mode: Optional[str] = None, points_to_evaluate: Optional[List[Dict]] = None, + parallel_num: int = 1, **kwargs): assert zoopt is not None, "ZOOpt not found - please install zoopt " \ "by `pip install -U zoopt`." @@ -178,6 +179,8 @@ class ZOOptSearch(Searcher): self.kwargs = kwargs + self.parallel_num = parallel_num + super(ZOOptSearch, self).__init__(metric=self._metric, mode=mode) if self._dim_dict: @@ -206,7 +209,10 @@ class ZOOptSearch(Searcher): if self._algo == "sracos" or self._algo == "asracos": from zoopt.algos.opt_algorithms.racos.sracos import SRacosTune self.optimizer = SRacosTune( - dimension=dim, parameter=par, **self.kwargs) + dimension=dim, + parameter=par, + parallel_num=self.parallel_num, + **self.kwargs) if init_samples: self.optimizer.init_attribute() diff --git a/python/ray/tune/tests/test_tune_restore.py b/python/ray/tune/tests/test_tune_restore.py index 507dca1d7..fc61c81ff 100644 --- a/python/ray/tune/tests/test_tune_restore.py +++ b/python/ray/tune/tests/test_tune_restore.py @@ -105,11 +105,6 @@ class TuneExampleTest(unittest.TestCase): validate_save_restore(TrainMNIST) validate_save_restore(TrainMNIST, use_object_store=True) - def testLogging(self): - from ray.tune.examples.logging_example import MyTrainableClass - validate_save_restore(MyTrainableClass) - validate_save_restore(MyTrainableClass, use_object_store=True) - def testHyperbandExample(self): from ray.tune.examples.hyperband_example import MyTrainableClass validate_save_restore(MyTrainableClass) @@ -373,22 +368,72 @@ class SigOptWarmStartTest(AbstractWarmStartTest, unittest.TestCase): def cost(space, reporter): reporter(loss=(space["height"] - 14)**2 - abs(space["width"] - 3)) + # Unfortunately, SigOpt doesn't allow setting of random state. Thus, + # we always end up with different suggestions, which is unsuitable + # for the warm start test. Here we make do with points_to_evaluate, + # and ensure that state is preserved over checkpoints and restarts. + points = [ + { + "width": 5, + "height": 20 + }, + { + "width": 10, + "height": -20 + }, + { + "width": 15, + "height": 30 + }, + { + "width": 5, + "height": -30 + }, + { + "width": 10, + "height": 40 + }, + { + "width": 15, + "height": -40 + }, + { + "width": 5, + "height": 50 + }, + { + "width": 10, + "height": -50 + }, + { + "width": 15, + "height": 60 + }, + { + "width": 12, + "height": -60 + }, + ] + search_alg = SigOptSearch( space, name="SigOpt Example Experiment", max_concurrent=1, metric="loss", - mode="min") + mode="min", + points_to_evaluate=points) return search_alg, cost def testWarmStart(self): - if ("SIGOPT_KEY" not in os.environ): + if "SIGOPT_KEY" not in os.environ: + self.skipTest("No SigOpt API key found in environment.") return super().testWarmStart() def testRestore(self): - if ("SIGOPT_KEY" not in os.environ): + if "SIGOPT_KEY" not in os.environ: + self.skipTest("No SigOpt API key found in environment.") return super().testRestore() @@ -412,10 +457,6 @@ class ZOOptWarmStartTest(AbstractWarmStartTest, unittest.TestCase): return search_alg, cost - @unittest.skip("Skip because this seems to have leaking state.") - def testRestore(self): - pass - class SearcherTest(unittest.TestCase): class MockSearcher(Searcher):