From b4fc182e6c0601b7abac247d5696b28a960ab27c Mon Sep 17 00:00:00 2001 From: yoshinobc Date: Sun, 24 Apr 2022 13:53:43 +0900 Subject: [PATCH 1/5] add get_cached_extra_study_property --- optuna_dashboard/_app.py | 7 ++- ...ace.py => _cached_extra_study_property.py} | 41 +++++++++------- optuna_dashboard/_intermediate_values.py | 49 ------------------- 3 files changed, 26 insertions(+), 71 deletions(-) rename optuna_dashboard/{_search_space.py => _cached_extra_study_property.py} (59%) delete mode 100644 optuna_dashboard/_intermediate_values.py diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 84c3a608..c6dd6ae3 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -35,9 +35,8 @@ from optuna.version import __version__ as optuna_ver from packaging import version from . import _note as note +from ._cached_extra_study_property import get_cached_extra_study_property from ._importance import get_param_importance_from_trials_cache -from ._intermediate_values import has_intermediate_values -from ._search_space import get_search_space from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary @@ -284,13 +283,13 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: response.status = 404 # Not found return {"reason": f"study_id={study_id} is not found"} trials = get_trials(storage, study_id) - intersection, union = get_search_space(study_id, trials) + intersection, union, has_intermeridate_values = get_cached_extra_study_property(study_id, trials) return serialize_study_detail( summary, trials[after:], intersection, union, - has_intermediate_values(study_id, trials), + has_intermeridate_values, ) @app.get("/api/studies//param_importances") diff --git a/optuna_dashboard/_search_space.py b/optuna_dashboard/_cached_extra_study_property.py similarity index 59% rename from optuna_dashboard/_search_space.py rename to optuna_dashboard/_cached_extra_study_property.py index ff7d9eb3..a3b76a96 100644 --- a/optuna_dashboard/_search_space.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -5,39 +5,36 @@ from typing import List from typing import Optional from typing import Set from typing import Tuple +from typing import Dict +from typing import List from optuna.distributions import BaseDistribution from optuna.trial import FrozenTrial from optuna.trial import TrialState - SearchSpaceSetT = Set[Tuple[str, BaseDistribution]] SearchSpaceListT = List[Tuple[str, BaseDistribution]] -# In-memory search space cache -search_space_cache_lock = threading.Lock() -search_space_cache: Dict[int, "_SearchSpace"] = {} +cached_extra_study_property_cache_lock = threading.Lock() +cached_extra_study_property_cache: Dict[int, "_CachedExtraStudyProperty"] = {} states_of_interest = [TrialState.COMPLETE, TrialState.PRUNED] +def get_cached_extra_study_property(study_id: int, trials: List[FrozenTrial]) -> Tuple[SearchSpaceListT, SearchSpaceListT, bool]: + with cached_extra_study_property_cache_lock: + cached_extra_study_property = cached_extra_study_property_cache.get(study_id, None) + if cached_extra_study_property is None: + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + cached_extra_study_property_cache[study_id] = cached_extra_study_property + return cached_extra_study_property.intersection, cached_extra_study_property.union, cached_extra_study_property.has_intermediate_values -def get_search_space( - study_id: int, trials: List[FrozenTrial] -) -> Tuple[SearchSpaceListT, SearchSpaceListT]: - with search_space_cache_lock: - search_space = search_space_cache.get(study_id, None) - if search_space is None: - search_space = _SearchSpace() - search_space.update(trials) - search_space_cache[study_id] = search_space - return search_space.intersection, search_space.union - - -class _SearchSpace: +class _CachedExtraStudyProperty: def __init__(self) -> None: self._cursor: int = -1 self._intersection: Optional[SearchSpaceSetT] = None self._union: SearchSpaceSetT = set() + self.has_intermediate_values: bool = False @property def intersection(self) -> SearchSpaceListT: @@ -54,17 +51,24 @@ class _SearchSpace: return union def update(self, trials: List[FrozenTrial]) -> None: + if self.has_intermediate_values: + return + next_cursor = self._cursor for trial in reversed(trials): if self._cursor > trial.number: break - if not trial.state.is_finished(): + if not trial.state.is_finished: next_cursor = trial.number if trial.state not in states_of_interest: continue + + if not self.has_intermediate_values and len(trial.intermediate_values) > 0 : + self.has_intermediate_values = True + current = set([(n, d) for n, d in trial.distributions.items()]) self._union = self._union.union(current) @@ -72,4 +76,5 @@ class _SearchSpace: self._intersection = copy.copy(current) else: self._intersection = self._intersection.intersection(current) + self._cursor = next_cursor diff --git a/optuna_dashboard/_intermediate_values.py b/optuna_dashboard/_intermediate_values.py deleted file mode 100644 index d9b8e463..00000000 --- a/optuna_dashboard/_intermediate_values.py +++ /dev/null @@ -1,49 +0,0 @@ -import threading -from typing import Dict -from typing import List - -from optuna.trial import FrozenTrial -from optuna.trial import TrialState - - -# In-memory cache -intermediate_values_cache_lock = threading.Lock() -intermediate_values_cache: Dict[int, "_IntermediateValues"] = {} -states_of_interest = [TrialState.COMPLETE, TrialState.PRUNED] - - -def has_intermediate_values(study_id: int, trials: List[FrozenTrial]) -> bool: - with intermediate_values_cache_lock: - intermediate_values = intermediate_values_cache.get(study_id, None) - if intermediate_values is None: - intermediate_values = _IntermediateValues() - intermediate_values.update(trials) - intermediate_values_cache[study_id] = intermediate_values - return intermediate_values.has_intermediate_values - - -class _IntermediateValues: - def __init__(self) -> None: - self._cursor: int = -1 - self.has_intermediate_values: bool = False - - def update(self, trials: List[FrozenTrial]) -> None: - if self.has_intermediate_values: - return - - next_cursor = self._cursor - for trial in reversed(trials): - if self._cursor > trial.number: - break - - if not trial.state.is_finished(): - next_cursor = trial.number - - if trial.state not in states_of_interest: - continue - - current = len(trial.intermediate_values) > 0 - if current: - self.has_intermediate_values = True - return - self._cursor = next_cursor From d08b4f2867b060981c374f6e40891e860eca5c30 Mon Sep 17 00:00:00 2001 From: yoshinobc Date: Sun, 24 Apr 2022 13:54:47 +0900 Subject: [PATCH 2/5] fix --- optuna_dashboard/_cached_extra_study_property.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index a3b76a96..c1f50d19 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -15,6 +15,7 @@ from optuna.trial import TrialState SearchSpaceSetT = Set[Tuple[str, BaseDistribution]] SearchSpaceListT = List[Tuple[str, BaseDistribution]] +# In-memory cache cached_extra_study_property_cache_lock = threading.Lock() cached_extra_study_property_cache: Dict[int, "_CachedExtraStudyProperty"] = {} @@ -59,7 +60,7 @@ class _CachedExtraStudyProperty: if self._cursor > trial.number: break - if not trial.state.is_finished: + if not trial.state.is_finished(): next_cursor = trial.number if trial.state not in states_of_interest: From 4c97e4a2899cca7749e6387b2666e1581b4496d4 Mon Sep 17 00:00:00 2001 From: yoshinobc Date: Sun, 24 Apr 2022 14:53:33 +0900 Subject: [PATCH 3/5] add python tests --- .../test_cached_extra_study_property.py | 177 ++++++++++++++++++ python_tests/test_search_space.py | 115 ------------ 2 files changed, 177 insertions(+), 115 deletions(-) create mode 100644 python_tests/test_cached_extra_study_property.py delete mode 100644 python_tests/test_search_space.py diff --git a/python_tests/test_cached_extra_study_property.py b/python_tests/test_cached_extra_study_property.py new file mode 100644 index 00000000..2ab8395a --- /dev/null +++ b/python_tests/test_cached_extra_study_property.py @@ -0,0 +1,177 @@ +from unittest import TestCase +import warnings + +import optuna +from optuna import create_trial +from optuna.distributions import UniformDistribution +from optuna.exceptions import ExperimentalWarning +from optuna.trial import TrialState + +from optuna_dashboard._cached_extra_study_property import _CachedExtraStudyProperty + + +class _CachedExtraStudyPropertyTestCase(TestCase): + def setUp(self) -> None: + optuna.logging.set_verbosity(optuna.logging.ERROR) + warnings.simplefilter("ignore", category=ExperimentalWarning) + + def test_same_distributions(self) -> None: + distributions = [ + { + "x0": UniformDistribution(low=0, high=10), + "x1": UniformDistribution(low=0, high=10), + }, + { + "x0": UniformDistribution(low=0, high=10), + "x1": UniformDistribution(low=0, high=10), + }, + ] + params = [ + { + "x0": 0.5, + "x1": 0.5, + }, + { + "x0": 0.5, + "x1": 0.5, + }, + ] + trials = [ + create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) + for d, p in zip(distributions, params) + ] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + + self.assertEqual(len(cached_extra_study_property.intersection), 2) + self.assertEqual(len(cached_extra_study_property.union), 2) + + def test_different_distributions(self) -> None: + distributions = [ + { + "x0": UniformDistribution(low=0, high=10), + "x1": UniformDistribution(low=0, high=10), + }, + { + "x0": UniformDistribution(low=0, high=5), + "x1": UniformDistribution(low=0, high=10), + }, + ] + params = [ + { + "x0": 0.5, + "x1": 0.5, + }, + { + "x0": 0.5, + "x1": 0.5, + }, + ] + trials = [ + create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) + for d, p in zip(distributions, params) + ] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + + self.assertEqual(len(cached_extra_study_property.intersection), 1) + self.assertEqual(len(cached_extra_study_property.union), 3) + + def test_dynamic_search_space(self) -> None: + distributions = [ + { + "x0": UniformDistribution(low=0, high=10), + "x1": UniformDistribution(low=0, high=10), + }, + { + "x0": UniformDistribution(low=0, high=5), + }, + { + "x0": UniformDistribution(low=0, high=10), + "x1": UniformDistribution(low=0, high=10), + }, + ] + params = [ + { + "x0": 0.5, + "x1": 0.5, + }, + { + "x0": 0.5, + }, + { + "x0": 0.5, + "x1": 0.5, + }, + ] + trials = [ + create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) + for d, p in zip(distributions, params) + ] + + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + + self.assertEqual(len(cached_extra_study_property.intersection), 0) + self.assertEqual(len(cached_extra_study_property.union), 3) + + def test_no_intermediate_value(self) -> None: + intermediate_values = [ + {}, + {}, + ] + trials = [ + create_trial( + state=TrialState.COMPLETE, + value=0, + distributions={"x0": UniformDistribution(low=0, high=10)}, + intermediate_values=iv, + params={"x0": 0.5}, + ) + for iv in intermediate_values + ] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + self.assertFalse(cached_extra_study_property.has_intermediate_values) + + def test_some_trials_has_no_intermediate_value(self) -> None: + intermediate_values = [ + {0: 0.3, 1: 1.2}, + {}, + {0: 0.3, 1: 1.2}, + ] + trials = [ + create_trial( + state=TrialState.COMPLETE, + value=0, + distributions={"x0": UniformDistribution(low=0, high=10)}, + intermediate_values=iv, + params={"x0": 0.5}, + ) + for iv in intermediate_values + ] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + self.assertTrue(cached_extra_study_property.has_intermediate_values) + + def test_all_trials_has_intermediate_value(self) -> None: + intermediate_values = [{0: 0.3, 1: 1.2}, {0: 0.3, 1: 1.2}] + trials = [ + create_trial( + state=TrialState.COMPLETE, + value=0, + distributions={"x0": UniformDistribution(low=0, high=10)}, + intermediate_values=iv, + params={"x0": 0.5}, + ) + for iv in intermediate_values + ] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + self.assertTrue(cached_extra_study_property.has_intermediate_values) + + def test_no_trials(self) -> None: + trials = [] + cached_extra_study_property = _CachedExtraStudyProperty() + cached_extra_study_property.update(trials) + self.assertFalse(cached_extra_study_property.has_intermediate_values) diff --git a/python_tests/test_search_space.py b/python_tests/test_search_space.py deleted file mode 100644 index df7fe9e2..00000000 --- a/python_tests/test_search_space.py +++ /dev/null @@ -1,115 +0,0 @@ -from unittest import TestCase -import warnings - -import optuna -from optuna import create_trial -from optuna.distributions import UniformDistribution -from optuna.exceptions import ExperimentalWarning -from optuna.trial import TrialState - -from optuna_dashboard._search_space import _SearchSpace - - -class SearchSpaceTestCase(TestCase): - def setUp(self) -> None: - optuna.logging.set_verbosity(optuna.logging.ERROR) - warnings.simplefilter("ignore", category=ExperimentalWarning) - - def test_same_distributions(self) -> None: - distributions = [ - { - "x0": UniformDistribution(low=0, high=10), - "x1": UniformDistribution(low=0, high=10), - }, - { - "x0": UniformDistribution(low=0, high=10), - "x1": UniformDistribution(low=0, high=10), - }, - ] - params = [ - { - "x0": 0.5, - "x1": 0.5, - }, - { - "x0": 0.5, - "x1": 0.5, - }, - ] - trials = [ - create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) - for d, p in zip(distributions, params) - ] - search_space = _SearchSpace() - search_space.update(trials) - - self.assertEqual(len(search_space.intersection), 2) - self.assertEqual(len(search_space.union), 2) - - def test_different_distributions(self) -> None: - distributions = [ - { - "x0": UniformDistribution(low=0, high=10), - "x1": UniformDistribution(low=0, high=10), - }, - { - "x0": UniformDistribution(low=0, high=5), - "x1": UniformDistribution(low=0, high=10), - }, - ] - params = [ - { - "x0": 0.5, - "x1": 0.5, - }, - { - "x0": 0.5, - "x1": 0.5, - }, - ] - trials = [ - create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) - for d, p in zip(distributions, params) - ] - search_space = _SearchSpace() - search_space.update(trials) - - self.assertEqual(len(search_space.intersection), 1) - self.assertEqual(len(search_space.union), 3) - - def test_dynamic_search_space(self) -> None: - distributions = [ - { - "x0": UniformDistribution(low=0, high=10), - "x1": UniformDistribution(low=0, high=10), - }, - { - "x0": UniformDistribution(low=0, high=5), - }, - { - "x0": UniformDistribution(low=0, high=10), - "x1": UniformDistribution(low=0, high=10), - }, - ] - params = [ - { - "x0": 0.5, - "x1": 0.5, - }, - { - "x0": 0.5, - }, - { - "x0": 0.5, - "x1": 0.5, - }, - ] - trials = [ - create_trial(state=TrialState.COMPLETE, value=0, distributions=d, params=p) - for d, p in zip(distributions, params) - ] - search_space = _SearchSpace() - search_space.update(trials) - - self.assertEqual(len(search_space.intersection), 0) - self.assertEqual(len(search_space.union), 3) From c8f7cf5de07acc2bbfab7c12eee2f10b71ed2340 Mon Sep 17 00:00:00 2001 From: yoshinobc Date: Sun, 24 Apr 2022 14:57:18 +0900 Subject: [PATCH 4/5] fix lint --- optuna_dashboard/_app.py | 4 ++- .../_cached_extra_study_property.py | 25 +++++++++++-------- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index c6dd6ae3..6b140839 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -283,7 +283,9 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: response.status = 404 # Not found return {"reason": f"study_id={study_id} is not found"} trials = get_trials(storage, study_id) - intersection, union, has_intermeridate_values = get_cached_extra_study_property(study_id, trials) + intersection, union, has_intermeridate_values = get_cached_extra_study_property( + study_id, trials + ) return serialize_study_detail( summary, trials[after:], diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index c1f50d19..e1c8d0ef 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -5,13 +5,12 @@ from typing import List from typing import Optional from typing import Set from typing import Tuple -from typing import Dict -from typing import List from optuna.distributions import BaseDistribution from optuna.trial import FrozenTrial from optuna.trial import TrialState + SearchSpaceSetT = Set[Tuple[str, BaseDistribution]] SearchSpaceListT = List[Tuple[str, BaseDistribution]] @@ -21,14 +20,24 @@ cached_extra_study_property_cache: Dict[int, "_CachedExtraStudyProperty"] = {} states_of_interest = [TrialState.COMPLETE, TrialState.PRUNED] -def get_cached_extra_study_property(study_id: int, trials: List[FrozenTrial]) -> Tuple[SearchSpaceListT, SearchSpaceListT, bool]: + +def get_cached_extra_study_property( + study_id: int, trials: List[FrozenTrial] +) -> Tuple[SearchSpaceListT, SearchSpaceListT, bool]: with cached_extra_study_property_cache_lock: - cached_extra_study_property = cached_extra_study_property_cache.get(study_id, None) + cached_extra_study_property = cached_extra_study_property_cache.get( + study_id, None + ) if cached_extra_study_property is None: cached_extra_study_property = _CachedExtraStudyProperty() cached_extra_study_property.update(trials) cached_extra_study_property_cache[study_id] = cached_extra_study_property - return cached_extra_study_property.intersection, cached_extra_study_property.union, cached_extra_study_property.has_intermediate_values + return ( + cached_extra_study_property.intersection, + cached_extra_study_property.union, + cached_extra_study_property.has_intermediate_values, + ) + class _CachedExtraStudyProperty: def __init__(self) -> None: @@ -52,9 +61,6 @@ class _CachedExtraStudyProperty: return union def update(self, trials: List[FrozenTrial]) -> None: - if self.has_intermediate_values: - return - next_cursor = self._cursor for trial in reversed(trials): if self._cursor > trial.number: @@ -66,8 +72,7 @@ class _CachedExtraStudyProperty: if trial.state not in states_of_interest: continue - - if not self.has_intermediate_values and len(trial.intermediate_values) > 0 : + if len(trial.intermediate_values) > 0: self.has_intermediate_values = True current = set([(n, d) for n, d in trial.distributions.items()]) From 71591c0a052e613ba338f403d71e1ee90ff9c464 Mon Sep 17 00:00:00 2001 From: yoshinobc Date: Sun, 24 Apr 2022 17:15:49 +0900 Subject: [PATCH 5/5] fix test and update --- optuna_dashboard/_cached_extra_study_property.py | 2 +- python_tests/test_cached_extra_study_property.py | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index e1c8d0ef..5945dfcc 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -72,7 +72,7 @@ class _CachedExtraStudyProperty: if trial.state not in states_of_interest: continue - if len(trial.intermediate_values) > 0: + if not self.has_intermediate_values and len(trial.intermediate_values) > 0: self.has_intermediate_values = True current = set([(n, d) for n, d in trial.distributions.items()]) diff --git a/python_tests/test_cached_extra_study_property.py b/python_tests/test_cached_extra_study_property.py index 2ab8395a..09e91c22 100644 --- a/python_tests/test_cached_extra_study_property.py +++ b/python_tests/test_cached_extra_study_property.py @@ -1,3 +1,5 @@ +from typing import Dict +from typing import List from unittest import TestCase import warnings @@ -10,7 +12,7 @@ from optuna.trial import TrialState from optuna_dashboard._cached_extra_study_property import _CachedExtraStudyProperty -class _CachedExtraStudyPropertyTestCase(TestCase): +class _CachedExtraStudyPropertySearchSpaceTestCase(TestCase): def setUp(self) -> None: optuna.logging.set_verbosity(optuna.logging.ERROR) warnings.simplefilter("ignore", category=ExperimentalWarning) @@ -115,8 +117,14 @@ class _CachedExtraStudyPropertyTestCase(TestCase): self.assertEqual(len(cached_extra_study_property.intersection), 0) self.assertEqual(len(cached_extra_study_property.union), 3) + +class _CachedExtraStudyPropertyIntermediateTestCase(TestCase): + def setUp(self) -> None: + optuna.logging.set_verbosity(optuna.logging.ERROR) + warnings.simplefilter("ignore", category=ExperimentalWarning) + def test_no_intermediate_value(self) -> None: - intermediate_values = [ + intermediate_values: List[Dict] = [ {}, {}, ] @@ -171,7 +179,7 @@ class _CachedExtraStudyPropertyTestCase(TestCase): self.assertTrue(cached_extra_study_property.has_intermediate_values) def test_no_trials(self) -> None: - trials = [] + trials: list = [] cached_extra_study_property = _CachedExtraStudyProperty() cached_extra_study_property.update(trials) self.assertFalse(cached_extra_study_property.has_intermediate_values)