From 884e9a3c54c39452d61492e0b6afadf36e77987a Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 14:00:15 +0900 Subject: [PATCH 01/10] modify /api/studies/ to return union_user_attrs --- optuna_dashboard/_app.py | 10 +++++++--- optuna_dashboard/_cached_extra_study_property.py | 14 +++++++++++++- optuna_dashboard/_serializer.py | 14 ++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 1c9e987a..23cafa78 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -297,14 +297,18 @@ 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, + union_user_attrs, + has_intermeridate_values, + ) = get_cached_extra_study_property(study_id, trials) return serialize_study_detail( summary, trials[after:], intersection, union, + union_user_attrs, has_intermeridate_values, ) diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index 3688a010..0046336c 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -1,5 +1,6 @@ import copy import threading +from typing import Any from typing import Dict from typing import List from typing import Optional @@ -23,7 +24,7 @@ states_of_interest = [TrialState.COMPLETE, TrialState.PRUNED] def get_cached_extra_study_property( study_id: int, trials: List[FrozenTrial] -) -> Tuple[SearchSpaceListT, SearchSpaceListT, bool]: +) -> Tuple[SearchSpaceListT, SearchSpaceListT, List[Tuple[str, Any]], 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: @@ -33,6 +34,7 @@ def get_cached_extra_study_property( return ( cached_extra_study_property.intersection, cached_extra_study_property.union, + cached_extra_study_property.union_user_attrs, cached_extra_study_property.has_intermediate_values, ) @@ -42,6 +44,7 @@ class _CachedExtraStudyProperty: self._cursor: int = -1 self._intersection: Optional[SearchSpaceSetT] = None self._union: SearchSpaceSetT = set() + self._union_user_attrs: Set[Tuple[str, Any]] = set() self.has_intermediate_values: bool = False @property @@ -58,6 +61,12 @@ class _CachedExtraStudyProperty: union.sort(key=lambda x: x[0]) return union + @property + def union_user_attrs(self) -> Set[Tuple[str, Any]]: + union = list(self._union_user_attrs) + union.sort(key=lambda x: x[0]) + return union + def update(self, trials: List[FrozenTrial]) -> None: next_cursor = self._cursor for trial in reversed(trials): @@ -81,4 +90,7 @@ class _CachedExtraStudyProperty: else: self._intersection = self._intersection.intersection(current) + current_user_attrs = set([(n, d) for n, d in trial.user_attrs.items()]) + self._union_user_attrs = self._union_user_attrs.union(current_user_attrs) + self._cursor = next_cursor diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index dc754e82..3f39b5e1 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -71,6 +71,7 @@ def serialize_study_detail( trials: List[FrozenTrial], intersection: List[Tuple[str, BaseDistribution]], union: List[Tuple[str, BaseDistribution]], + union_user_attrs: List[Tuple[str, Any]], has_intermediate_values: bool, ) -> Dict[str, Any]: serialized: Dict[str, Any] = { @@ -83,6 +84,7 @@ def serialize_study_detail( serialized["trials"] = [serialize_frozen_trial(summary._study_id, trial) for trial in trials] serialized["intersection_search_space"] = serialize_search_space(intersection) serialized["union_search_space"] = serialize_search_space(union) + serialized["union_user_attrs"] = serialize_user_attrs(union_user_attrs) serialized["has_intermediate_values"] = has_intermediate_values serialized["note"] = note.get_note_from_system_attrs(summary.system_attrs) return serialized @@ -150,3 +152,15 @@ def serialize_search_space( } ) return serialized + + +def serialize_user_attrs(user_attrs: List[Tuple[str, Any]]) -> List[Dict[str, Any]]: + serialized = [] + for attr_name, attr_value in user_attrs: + serialized.append( + { + "name": attr_name, + "value": attr_value, + } + ) + return serialized From 11eb1fb99821a3f4a5248ad22fab78abfaf77411 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 14:05:02 +0900 Subject: [PATCH 02/10] receive union_user_attrs from /api/studies/ --- optuna_dashboard/ts/apiClient.ts | 2 ++ optuna_dashboard/ts/types/index.d.ts | 1 + 2 files changed, 3 insertions(+) diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index d0f30038..3f45b7bd 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -44,6 +44,7 @@ interface StudyDetailResponse { trials: TrialResponse[] intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] + union_user_attrs: Attribute[] has_intermediate_values: boolean note: { version: number @@ -73,6 +74,7 @@ export const getStudyDetailAPI = ( trials: trials, union_search_space: res.data.union_search_space, intersection_search_space: res.data.intersection_search_space, + union_user_attrs: res.data.union_user_attrs, has_intermediate_values: res.data.has_intermediate_values, note: res.data.note, } diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index 951e858e..b4b576f5 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -84,6 +84,7 @@ declare interface StudyDetail { trials: Trial[] intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] + union_user_attrs: Attribute[] has_intermediate_values: boolean note: Note } From 431b0fc835eb4e08b1c1051236f3c1e0d9700362 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 14:05:40 +0900 Subject: [PATCH 03/10] add simple user attributes column --- optuna_dashboard/ts/components/StudyDetail.tsx | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/optuna_dashboard/ts/components/StudyDetail.tsx b/optuna_dashboard/ts/components/StudyDetail.tsx index feb9a88c..fed6c3e3 100644 --- a/optuna_dashboard/ts/components/StudyDetail.tsx +++ b/optuna_dashboard/ts/components/StudyDetail.tsx @@ -650,6 +650,13 @@ export const TrialTable: FC<{ studyDetail: StudyDetail | null }> = ({ }) } + columns.push({ + field: "user_attrs", + label: "User Attributes", + toCellValue: (i) => + trials[i].user_attrs.map((p) => p.key + ": " + p.value).join(", "), + }) + const collapseParamColumns: DataGridColumn[] = [ { field: "name", label: "Name", sortable: true }, { field: "value", label: "Value", sortable: true }, From 410e27c5790fccd2d44d2ead16812a0cfb358bd5 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 15:13:34 +0900 Subject: [PATCH 04/10] fix a mypy error --- optuna_dashboard/_cached_extra_study_property.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index 0046336c..2ff86901 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -62,7 +62,7 @@ class _CachedExtraStudyProperty: return union @property - def union_user_attrs(self) -> Set[Tuple[str, Any]]: + def union_user_attrs(self) -> List[Tuple[str, Any]]: union = list(self._union_user_attrs) union.sort(key=lambda x: x[0]) return union From b765f3280754aa9d39e7184a2dd987a714938777 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 16:02:36 +0900 Subject: [PATCH 05/10] fix a typescript test error --- typescript_tests/TrialTable.test.tsx | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/typescript_tests/TrialTable.test.tsx b/typescript_tests/TrialTable.test.tsx index da783fec..406e50fb 100644 --- a/typescript_tests/TrialTable.test.tsx +++ b/typescript_tests/TrialTable.test.tsx @@ -74,6 +74,16 @@ const studyDetail = { attributes: { low: -3, high: 3 }, }, ], + union_user_attrs: [ + { + key: "foo", + value: "hoge", + }, + { + key: "bar", + value: "0", + }, + ], has_intermediate_values: false, note: { version: 0, From e33481572c1e59f92577975174e678a9c7c5ef8c Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 16:24:27 +0900 Subject: [PATCH 06/10] modify union_user_attrs to hold only keys --- optuna_dashboard/_cached_extra_study_property.py | 11 +++++------ optuna_dashboard/_serializer.py | 16 ++-------------- optuna_dashboard/ts/apiClient.ts | 2 +- optuna_dashboard/ts/types/index.d.ts | 2 +- typescript_tests/TrialTable.test.tsx | 11 +---------- 5 files changed, 10 insertions(+), 32 deletions(-) diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index 2ff86901..13b3340f 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -1,6 +1,5 @@ import copy import threading -from typing import Any from typing import Dict from typing import List from typing import Optional @@ -24,7 +23,7 @@ states_of_interest = [TrialState.COMPLETE, TrialState.PRUNED] def get_cached_extra_study_property( study_id: int, trials: List[FrozenTrial] -) -> Tuple[SearchSpaceListT, SearchSpaceListT, List[Tuple[str, Any]], bool]: +) -> Tuple[SearchSpaceListT, SearchSpaceListT, List[str], 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: @@ -44,7 +43,7 @@ class _CachedExtraStudyProperty: self._cursor: int = -1 self._intersection: Optional[SearchSpaceSetT] = None self._union: SearchSpaceSetT = set() - self._union_user_attrs: Set[Tuple[str, Any]] = set() + self._union_user_attrs: Set[str] = set() self.has_intermediate_values: bool = False @property @@ -62,9 +61,9 @@ class _CachedExtraStudyProperty: return union @property - def union_user_attrs(self) -> List[Tuple[str, Any]]: + def union_user_attrs(self) -> List[str]: union = list(self._union_user_attrs) - union.sort(key=lambda x: x[0]) + union.sort() return union def update(self, trials: List[FrozenTrial]) -> None: @@ -90,7 +89,7 @@ class _CachedExtraStudyProperty: else: self._intersection = self._intersection.intersection(current) - current_user_attrs = set([(n, d) for n, d in trial.user_attrs.items()]) + current_user_attrs = set(n for n in trial.user_attrs.keys()) self._union_user_attrs = self._union_user_attrs.union(current_user_attrs) self._cursor = next_cursor diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 3f39b5e1..9dca618e 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -71,7 +71,7 @@ def serialize_study_detail( trials: List[FrozenTrial], intersection: List[Tuple[str, BaseDistribution]], union: List[Tuple[str, BaseDistribution]], - union_user_attrs: List[Tuple[str, Any]], + union_user_attrs: List[str], has_intermediate_values: bool, ) -> Dict[str, Any]: serialized: Dict[str, Any] = { @@ -84,7 +84,7 @@ def serialize_study_detail( serialized["trials"] = [serialize_frozen_trial(summary._study_id, trial) for trial in trials] serialized["intersection_search_space"] = serialize_search_space(intersection) serialized["union_search_space"] = serialize_search_space(union) - serialized["union_user_attrs"] = serialize_user_attrs(union_user_attrs) + serialized["union_user_attrs"] = union_user_attrs serialized["has_intermediate_values"] = has_intermediate_values serialized["note"] = note.get_note_from_system_attrs(summary.system_attrs) return serialized @@ -152,15 +152,3 @@ def serialize_search_space( } ) return serialized - - -def serialize_user_attrs(user_attrs: List[Tuple[str, Any]]) -> List[Dict[str, Any]]: - serialized = [] - for attr_name, attr_value in user_attrs: - serialized.append( - { - "name": attr_name, - "value": attr_value, - } - ) - return serialized diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index 3f45b7bd..2f00395b 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -44,7 +44,7 @@ interface StudyDetailResponse { trials: TrialResponse[] intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] - union_user_attrs: Attribute[] + union_user_attrs: string[] has_intermediate_values: boolean note: { version: number diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index b4b576f5..2f15ea75 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -84,7 +84,7 @@ declare interface StudyDetail { trials: Trial[] intersection_search_space: SearchSpace[] union_search_space: SearchSpace[] - union_user_attrs: Attribute[] + union_user_attrs: string[] has_intermediate_values: boolean note: Note } diff --git a/typescript_tests/TrialTable.test.tsx b/typescript_tests/TrialTable.test.tsx index 406e50fb..be9dd663 100644 --- a/typescript_tests/TrialTable.test.tsx +++ b/typescript_tests/TrialTable.test.tsx @@ -74,16 +74,7 @@ const studyDetail = { attributes: { low: -3, high: 3 }, }, ], - union_user_attrs: [ - { - key: "foo", - value: "hoge", - }, - { - key: "bar", - value: "0", - }, - ], + union_user_attrs: ["foo", "bar"], has_intermediate_values: false, note: { version: 0, From 31819f8c1dd593cf90a7e2de413440bad06ef63a Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 16:26:52 +0900 Subject: [PATCH 07/10] use one column for each user attribute --- .../ts/components/StudyDetail.tsx | 33 ++++++++++++++++--- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/optuna_dashboard/ts/components/StudyDetail.tsx b/optuna_dashboard/ts/components/StudyDetail.tsx index fed6c3e3..0b3cddb2 100644 --- a/optuna_dashboard/ts/components/StudyDetail.tsx +++ b/optuna_dashboard/ts/components/StudyDetail.tsx @@ -650,11 +650,34 @@ export const TrialTable: FC<{ studyDetail: StudyDetail | null }> = ({ }) } - columns.push({ - field: "user_attrs", - label: "User Attributes", - toCellValue: (i) => - trials[i].user_attrs.map((p) => p.key + ": " + p.value).join(", "), + studyDetail?.union_user_attrs.forEach((attr_name) => { + columns.push({ + field: "user_attrs", + label: `User attribute ${attr_name}`, + toCellValue: (i) => + trials[i].user_attrs.find((attr) => attr.key === attr_name)?.value || + null, + sortable: true, + filterable: true, + less: (firstEl, secondEl): number => { + const firstVal = firstEl.params.find( + (attr) => attr.key === attr_name + )?.value + const secondVal = secondEl.params.find( + (attr) => attr.key === attr_name + )?.value + + if (firstVal === secondVal) { + return 0 + } else if (firstVal && secondVal) { + return firstVal < secondVal ? 1 : -1 + } else if (firstVal) { + return -1 + } else { + return 1 + } + }, + }) }) const collapseParamColumns: DataGridColumn[] = [ From b0fbb9963cf72a82cbd89cf0fa9f17e2789a9619 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 17:03:24 +0900 Subject: [PATCH 08/10] fix a bug in sorting user_attrs columns --- optuna_dashboard/ts/components/StudyDetail.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optuna_dashboard/ts/components/StudyDetail.tsx b/optuna_dashboard/ts/components/StudyDetail.tsx index 0b3cddb2..2a52e47e 100644 --- a/optuna_dashboard/ts/components/StudyDetail.tsx +++ b/optuna_dashboard/ts/components/StudyDetail.tsx @@ -660,10 +660,10 @@ export const TrialTable: FC<{ studyDetail: StudyDetail | null }> = ({ sortable: true, filterable: true, less: (firstEl, secondEl): number => { - const firstVal = firstEl.params.find( + const firstVal = firstEl.user_attrs.find( (attr) => attr.key === attr_name )?.value - const secondVal = secondEl.params.find( + const secondVal = secondEl.user_attrs.find( (attr) => attr.key === attr_name )?.value From 303b887b82538b6c0a50009446d37c69af496513 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 17:28:31 +0900 Subject: [PATCH 09/10] Add TODO comments based on code review --- optuna_dashboard/_app.py | 1 + optuna_dashboard/_cached_extra_study_property.py | 1 + 2 files changed, 2 insertions(+) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 23cafa78..aa6e082f 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -298,6 +298,7 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": f"study_id={study_id} is not found"} trials = get_trials(storage, study_id) ( + # TODO: intersection_search_space and union_search_space look more clear since now we have union_user_attrs. intersection, union, union_user_attrs, diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index 13b3340f..5c673fc6 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -41,6 +41,7 @@ def get_cached_extra_study_property( class _CachedExtraStudyProperty: def __init__(self) -> None: self._cursor: int = -1 + # TODO: intersection_search_space and union_search_space look more clear since now we have union_user_attrs. self._intersection: Optional[SearchSpaceSetT] = None self._union: SearchSpaceSetT = set() self._union_user_attrs: Set[str] = set() From b29ca195fb6676666523e281771c14baec53b669 Mon Sep 17 00:00:00 2001 From: Masahiro Sakai Date: Sun, 6 Nov 2022 17:47:43 +0900 Subject: [PATCH 10/10] fix flake8 errors --- optuna_dashboard/_app.py | 3 ++- optuna_dashboard/_cached_extra_study_property.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index aa6e082f..98571add 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -298,7 +298,8 @@ def create_app(storage: BaseStorage, debug: bool = False) -> Bottle: return {"reason": f"study_id={study_id} is not found"} trials = get_trials(storage, study_id) ( - # TODO: intersection_search_space and union_search_space look more clear since now we have union_user_attrs. + # TODO: intersection_search_space and union_search_space look more clear since now we + # have union_user_attrs. intersection, union, union_user_attrs, diff --git a/optuna_dashboard/_cached_extra_study_property.py b/optuna_dashboard/_cached_extra_study_property.py index 5c673fc6..316e6d61 100644 --- a/optuna_dashboard/_cached_extra_study_property.py +++ b/optuna_dashboard/_cached_extra_study_property.py @@ -41,7 +41,8 @@ def get_cached_extra_study_property( class _CachedExtraStudyProperty: def __init__(self) -> None: self._cursor: int = -1 - # TODO: intersection_search_space and union_search_space look more clear since now we have union_user_attrs. + # TODO: intersection_search_space and union_search_space look more clear since now we have + # union_user_attrs. self._intersection: Optional[SearchSpaceSetT] = None self._union: SearchSpaceSetT = set() self._union_user_attrs: Set[str] = set()