From dbffb4cb6b48ff586f3d434ad3eab20bfe47708a Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 1 Sep 2023 16:33:57 +0900 Subject: [PATCH 01/22] add undo --- optuna_dashboard/_app.py | 16 ++++ optuna_dashboard/_preferential_history.py | 76 ++++++++++++++----- optuna_dashboard/_serializer.py | 4 +- .../preferential/_system_attrs.py | 5 ++ optuna_dashboard/ts/action.ts | 16 ++++ optuna_dashboard/ts/apiClient.ts | 16 ++++ .../ts/components/PreferenceHistory.tsx | 52 ++++++++++--- optuna_dashboard/ts/types/index.d.ts | 1 + python_tests/test_preferential_history.py | 4 +- 9 files changed, 158 insertions(+), 32 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index f582d5e5..0b69dfc7 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -28,6 +28,7 @@ from ._cached_extra_study_property import get_cached_extra_study_property from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials from ._preferential_history import report_history +from ._preferential_history import switching_history from ._rdb_migration import register_rdb_migration_route from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary @@ -296,6 +297,21 @@ def create_app( response.status = 204 return {} + @app.put("/api/studies//preference/") + @json_api_view + def switch_preference(study_id: int, history_uuid: str) -> dict[str, Any]: + try: + enable = request.json.get("enable", None) + if enable is None or not isinstance(enable, bool): + raise ValueError + except ValueError: + response.status = 400 + return {"reason": "Invalid request."} + switching_history(study_id, storage, history_uuid, enable) + + response.status = 204 + return {} + @app.post("/api/trials//tell") @json_api_view def tell_trial(trial_id: int) -> dict[str, Any]: diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 5c930932..dff10885 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -1,8 +1,8 @@ from __future__ import annotations from dataclasses import dataclass +from dataclasses import field from datetime import datetime -import json from typing import Any from typing import Literal from typing import TYPE_CHECKING @@ -10,6 +10,8 @@ import uuid from optuna.storages import BaseStorage +from .preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE +from .preferential._system_attrs import get_preference from .preferential._system_attrs import report_preferences @@ -28,7 +30,7 @@ if TYPE_CHECKING: ) -@dataclass(frozen=True) +@dataclass class ChooseWorstHistory: mode: Literal["ChooseWorst"] uuid: str @@ -36,6 +38,8 @@ class ChooseWorstHistory: timestamp: datetime candidates: list[int] # a list of trial number clicked: int # The worst trial number in the candidates. + evacuated_preference: list[tuple[int, int]] = field(default_factory=list) + # When undo the preference, this is used. Otherwise, this must be empty. def to_dict(self) -> dict[str, Any]: return { @@ -45,6 +49,8 @@ class ChooseWorstHistory: "timestamp": self.timestamp.isoformat(), "candidates": self.candidates, "clicked": self.clicked, + "enabled": len(self.evacuated_preference) == 0, + "evacuated_preference": self.evacuated_preference, } @@ -87,31 +93,65 @@ def report_history( storage.set_study_system_attr( study_id=study_id, key=key, - value=json.dumps(history.to_dict()), + value=history.to_dict(), ) -def serialize_preference_history( +def _load_preference_history(value: Any) -> History: + choice: dict[str, Any] = value + if choice["mode"] == "ChooseWorst": + return ChooseWorstHistory( + mode="ChooseWorst", + uuid=choice["uuid"], + preference_uuid=choice["preference_uuid"], + timestamp=datetime.fromisoformat(choice["timestamp"]), + candidates=choice["candidates"], + clicked=choice["clicked"], + evacuated_preference=choice["evacuated_preference"], + ) + else: + assert False, f"Unknown mode: {choice['mode']}" + + +def load_preference_history( + uuid: str, + system_attrs: dict[str, Any], +) -> History: + value = system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, []) + return _load_preference_history(value) + + +def serialize_preference_histories( system_attrs: dict[str, Any], ) -> list[dict[str, Any]]: histories: list[History] = [] for k, v in system_attrs.items(): if not k.startswith(_SYSTEM_ATTR_PREFIX_HISTORY): continue - choice: dict[str, Any] = json.loads(v) - if choice["mode"] == "ChooseWorst": - histories.append( - ChooseWorstHistory( - mode="ChooseWorst", - uuid=choice["uuid"], - preference_uuid=choice["preference_uuid"], - timestamp=datetime.fromisoformat(choice["timestamp"]), - candidates=choice["candidates"], - clicked=choice["clicked"], - ) - ) - else: - assert False, f"Unknown mode: {choice['mode']}" + histories.append(_load_preference_history(v)) histories.sort(key=lambda c: c.timestamp) return [history.to_dict() for history in histories] + + +def switching_history(study_id: int, storage: BaseStorage, uuid: str, enable: bool) -> None: + system_attrs = storage.get_study_system_attrs(study_id) + history = load_preference_history(uuid, system_attrs) + preference = get_preference(study_id, storage, history.preference_uuid) + print(history, preference, enable) + if enable and (len(preference) > 0 or len(history.evacuated_preference) == 0): + return + if (not enable) and (len(preference) == 0 or len(history.evacuated_preference) > 0): + return + history.evacuated_preference, preference = preference, history.evacuated_preference + print(history.to_dict(), preference) + storage.set_study_system_attr( + study_id=study_id, + key=_SYSTEM_ATTR_PREFIX_HISTORY + history.uuid, + value=history.to_dict(), + ) + storage.set_study_system_attr( + study_id=study_id, + key=_SYSTEM_ATTR_PREFIX_PREFERENCE + history.preference_uuid, + value=preference, + ) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 19ceb19b..fc040424 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -14,7 +14,7 @@ from optuna.trial import FrozenTrial from . import _note as note from ._form_widget import get_form_widgets_json from ._named_objectives import get_objective_names -from ._preferential_history import serialize_preference_history +from ._preferential_history import serialize_preference_histories from .artifact._backend import list_trial_artifacts from .preferential._study import _SYSTEM_ATTR_PREFERENTIAL_STUDY @@ -157,7 +157,7 @@ def serialize_study_detail( if form_widgets: serialized["form_widgets"] = form_widgets if serialized["is_preferential"]: - serialized["preference_history"] = serialize_preference_history(system_attrs) + serialized["preference_history"] = serialize_preference_histories(system_attrs) return serialized diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 00347d7e..4e3f55f5 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -34,6 +34,11 @@ def report_preferences( return preference_uuid +def get_preference(study_id: int, storage: BaseStorage, uuid: str) -> list[tuple[int, int]]: + system_attrs = storage.get_study_system_attrs(study_id) + return system_attrs.get(_SYSTEM_ATTR_PREFIX_PREFERENCE + uuid, []) # type: ignore + + def get_preferences( study_id: int, storage: BaseStorage, diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index e1896178..f42a17bb 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -16,6 +16,7 @@ import { deleteArtifactAPI, reportPreferenceAPI, skipPreferentialTrialAPI, + switchPreferentialHistoryAPI, } from "./apiClient" import { graphVisibilityState, @@ -609,6 +610,20 @@ export const actionCreator = () => { }) } + const switchPreferentialHistory = ( + studyId: number, + historyUuid: string, + enable: boolean + ) => { + switchPreferentialHistoryAPI(studyId, historyUuid, enable).catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { + variant: "error", + }) + console.log(err) + }) + } + return { updateAPIMeta, updateStudyDetail, @@ -630,6 +645,7 @@ export const actionCreator = () => { saveTrialUserAttrs, updatePreference, skipPreferentialTrial, + switchPreferentialHistory, } } diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index 9c72f739..6886f8b4 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -61,6 +61,7 @@ interface PreferenceChoiceResponce { clicked: number mode: PreferenceFeedbackMode timestamp: string + enabled: boolean } const convertPreferenceChoice = ( @@ -72,6 +73,7 @@ const convertPreferenceChoice = ( clicked: res.clicked, feedback_mode: res.mode, timestamp: new Date(res.timestamp), + enabled: res.enabled, } } @@ -362,3 +364,17 @@ export const skipPreferentialTrialAPI = ( return }) } + +export const switchPreferentialHistoryAPI = ( + studyId: number, + historyUuid: string, + enable: boolean +): Promise => { + return axiosInstance + .put(`/api/studies/${studyId}/preference/${historyUuid}`, { + enable: enable, + }) + .then(() => { + return + }) +} diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index e95a4cb1..76725faa 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -9,12 +9,15 @@ import { } from "@mui/material" import ClearIcon from "@mui/icons-material/Clear" import IconButton from "@mui/material/IconButton" +import UndoIcon from "@mui/icons-material/Undo" +import RedoIcon from "@mui/icons-material/Redo" import OpenInFullIcon from "@mui/icons-material/OpenInFull" import Modal from "@mui/material/Modal" import { TrialListDetail } from "./TrialList" import { MarkdownRenderer } from "./Note" import { red } from "@mui/material/colors" +import { actionCreator } from "../action" type TrialType = "worst" | "none" @@ -133,24 +136,52 @@ const CandidateTrial: FC<{ ) } -const ChoiceTrials: FC<{ choice: PreferenceChoice; trials: Trial[] }> = ({ - choice, - trials, -}) => { +const ChoiceTrials: FC<{ + choice: PreferenceChoice + trials: Trial[] + study_id: number +}> = ({ choice, trials, study_id }) => { const theme = useTheme() const worst_trials = new Set([choice.clicked]) + const actions = actionCreator() + const handleUndo = () => { + actions.switchPreferentialHistory(study_id, choice.uuid, false) + } + const handleRedo = () => { + actions.switchPreferentialHistory(study_id, choice.uuid, true) + } return ( - - {choice.timestamp.toISOString()} - + + {choice.timestamp.toLocaleString()} + + + + + + + + = ({ key={choice.uuid} choice={choice} trials={studyDetail.trials} + study_id={studyDetail.id} /> ))} diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index da90ad1d..26c08d38 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -215,4 +215,5 @@ type PreferenceChoice = { clicked: number feedback_mode: PreferenceFeedbackMode timestamp: Date + enabled: boolean } diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index 64591783..85f25374 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -3,7 +3,7 @@ from __future__ import annotations from typing import Callable from optuna_dashboard._preferential_history import report_history -from optuna_dashboard._serializer import serialize_preference_history +from optuna_dashboard._serializer import serialize_preference_histories from optuna_dashboard.preferential import create_study from optuna_dashboard.preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE @@ -40,7 +40,7 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) "clicked": 0, }, ) - history = serialize_preference_history(storage.get_study_system_attrs(study_id)) + history = serialize_preference_histories(storage.get_study_system_attrs(study_id)) sys_attrs = storage.get_study_system_attrs(study_id) assert len(history) == 2 assert history[0]["candidates"] == [0, 1, 2] From f4dea90e07f0403c36a86ee8ca75fe545696f75c Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 1 Sep 2023 17:27:14 +0900 Subject: [PATCH 02/22] add test --- optuna_dashboard/_preferential_history.py | 3 +- python_tests/test_api.py | 71 +++++++++++++++++++++++ python_tests/test_preferential_history.py | 58 ++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index dff10885..f95d57d3 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -61,7 +61,7 @@ def report_history( study_id: int, storage: BaseStorage, input_data: NewHistoryJSON, -) -> None: +) -> str: preferences = [] if input_data["mode"] == "ChooseWorst": preferences = [ @@ -95,6 +95,7 @@ def report_history( key=key, value=history.to_dict(), ) + return history_uuid def _load_preference_history(value: Any) -> History: diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 7d732e37..1fc92508 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -8,6 +8,7 @@ from optuna import get_all_study_summaries from optuna.study import StudyDirection from optuna_dashboard._app import create_app from optuna_dashboard._app import create_new_study +from optuna_dashboard._preferential_history import serialize_preference_histories from optuna_dashboard.preferential import create_study from .wsgi_client import send_request @@ -181,6 +182,76 @@ class APITestCase(TestCase): assert best_trials[0].number == 0 assert best_trials[1].number == 2 + def test_undo_redo_history(self) -> None: + storage = optuna.storages.InMemoryStorage() + study = create_study(storage=storage) + for _ in range(3): + trial = study.ask() + study.mark_comparison_ready(trial) + + app = create_app(storage) + study_id = study._study._study_id + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference", + "POST", + body=json.dumps( + { + "mode": "ChooseWorst", + "candidates": [0, 1, 2], + "clicked": 2, + } + ), + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert histories[0]["enabled"] + + history_uuid = histories[0]["uuid"] + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference/{history_uuid}", + "PUT", + body=json.dumps( + { + "enable": False, + } + ), + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert not histories[0]["enabled"] + assert len(study.get_preferences()) == 0 + + status, _, _ = send_request( + app, + f"/api/studies/{study_id}/preference/{history_uuid}", + "PUT", + body=json.dumps( + { + "enable": True, + } + ), + content_type="application/json", + ) + self.assertEqual(status, 204) + histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + assert len(histories) == 1 + assert histories[0]["enabled"] + preferences = study.get_preferences() + preferences.sort(key=lambda x: (x[0].number, x[1].number)) + assert len(preferences) == 2 + better, worse = preferences[0] + assert better.number == 0 + assert worse.number == 2 + better, worse = preferences[1] + assert better.number == 1 + assert worse.number == 2 + def test_create_study(self) -> None: for name, directions, expected_status in [ ("single-objective success", ["minimize"], 201), diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index 85f25374..7b288968 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -2,10 +2,13 @@ from __future__ import annotations from typing import Callable +from optuna_dashboard._preferential_history import load_preference_history from optuna_dashboard._preferential_history import report_history +from optuna_dashboard._preferential_history import switching_history from optuna_dashboard._serializer import serialize_preference_histories from optuna_dashboard.preferential import create_study from optuna_dashboard.preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE +from optuna_dashboard.preferential._system_attrs import get_preference from .storage_supplier import parametrize_storages from .storage_supplier import StorageSupplier @@ -59,3 +62,58 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert len(preferences[i]) == 2 assert preferences[i][0] == best assert preferences[i][1] == worst + + +@parametrize_storages +def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> None: + with storage_supplier() as storage: + study = create_study(storage=storage) + for _ in range(5): + trial = study.ask() + trial.suggest_float("x", 0, 1) + study.mark_comparison_ready(trial) + + study_id = study._study._study_id + + history_uuid = report_history( + study_id=study_id, + storage=storage, + input_data={ + "mode": "ChooseWorst", + "candidates": [0, 1, 2], + "clicked": 1, + }, + ) + switching_history(study_id, storage, history_uuid, False) + history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) + preference = get_preference(study_id, storage, history.preference_uuid) + assert history.mode == "ChooseWorst" + assert history.candidates == [0, 1, 2] + assert history.clicked == 1 + assert len(history.evacuated_preference) == 2 + assert len(preference) == 0 + + switching_history(study_id, storage, history_uuid, False) + history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) + preference = get_preference(study_id, storage, history.preference_uuid) + assert len(history.evacuated_preference) == 2 + assert len(preference) == 0 + + switching_history(study_id, storage, history_uuid, True) + history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) + preference = get_preference(study_id, storage, history.preference_uuid) + assert history.mode == "ChooseWorst" + assert history.candidates == [0, 1, 2] + assert history.clicked == 1 + assert len(history.evacuated_preference) == 0 + assert len(preference) == 2 + for i, (best, worst) in enumerate([(0, 1), (2, 1)]): + assert len(preference[i]) == 2 + assert preference[i][0] == best + assert preference[i][1] == worst + + switching_history(study_id, storage, history_uuid, True) + history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) + preference = get_preference(study_id, storage, history.preference_uuid) + assert len(history.evacuated_preference) == 0 + assert len(preference) == 2 From a29f25bfd87d621097d95b041ade07e66f6b7d1e Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 8 Sep 2023 15:23:05 +0900 Subject: [PATCH 03/22] fix tests --- optuna_dashboard/_preferential_history.py | 10 +- optuna_dashboard/_serializer.py | 2 +- .../preferential/_system_attrs.py | 2 +- .../ts/components/PreferenceHistory.tsx | 41 +++++-- python_tests/test_api.py | 19 ++- python_tests/test_preferential_history.py | 113 +++++++++--------- 6 files changed, 101 insertions(+), 86 deletions(-) diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 3b9cd3e0..a8fe2148 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -8,7 +8,8 @@ import uuid from optuna.storages import BaseStorage -from .preferential._system_attrs import report_preferences, _SYSTEM_ATTR_PREFIX_PREFERENCE +from .preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE +from .preferential._system_attrs import report_preferences _SYSTEM_ATTR_PREFIX_HISTORY = "preference:history" @@ -43,7 +44,7 @@ def report_history( study_id: int, storage: BaseStorage, input_data: NewHistory, -) -> None: +) -> str: preferences = [] # TODO(moririn): Use TypeGuard after adding other history types. if input_data.mode == "ChooseWorst": @@ -78,14 +79,15 @@ def report_history( key=key, value=json.dumps(history), ) + return history_id def switching_history(study_id: int, storage: BaseStorage, uuid: str, enable: bool) -> None: system_attrs = storage.get_study_system_attrs(study_id) - history: History = system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, None) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, "")) if enable: preferences = [ - (best, history["clickedx"]) + (best, history["clicked"]) for best in history["candidates"] if best != history["clicked"] ] diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index cc9401b7..bcbe1ddf 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -187,7 +187,7 @@ def serialize_preference_history( "timestamp": choice["timestamp"], "candidates": choice["candidates"], "clicked": choice["clicked"], - "enabled": is_preference_valid(choice["preference_id"]), + "enabled": is_preference_valid(system_attrs, choice["preference_id"]), } histories.append(history) diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 0c49a8a8..5ce83742 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -47,7 +47,7 @@ def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]] def is_preference_valid(study_system_attrs: dict[str, Any], uuid: str) -> bool: key = _SYSTEM_ATTR_PREFIX_PREFERENCE + uuid preference = study_system_attrs.get(key, []) - return len(preference) == 0 + return len(preference) > 0 def report_skip( diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index c9444237..bfacfc62 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -9,9 +9,9 @@ import { } from "@mui/material" import ClearIcon from "@mui/icons-material/Clear" import IconButton from "@mui/material/IconButton" -import UndoIcon from "@mui/icons-material/Undo" -import RedoIcon from "@mui/icons-material/Redo" import OpenInFullIcon from "@mui/icons-material/OpenInFull" +import RestoreFromTrashIcon from "@mui/icons-material/RestoreFromTrash" +import DeleteIcon from "@mui/icons-material/Delete" import Modal from "@mui/material/Modal" import { red } from "@mui/material/colors" @@ -142,14 +142,13 @@ const ChoiceTrials: FC<{ trials: Trial[] study_id: number }> = ({ choice, trials, study_id }) => { + const [enabled, setEnabled] = useState(choice.enabled) const theme = useTheme() const worst_trials = new Set([choice.clicked]) const actions = actionCreator() - const handleUndo = () => { - actions.switchPreferentialHistory(study_id, choice.id, false) - } - const handleRedo = () => { - actions.switchPreferentialHistory(study_id, choice.id, true) + const handleSwitch = () => { + setEnabled(!enabled) + actions.switchPreferentialHistory(study_id, choice.id, !enabled) } return ( @@ -158,14 +157,32 @@ const ChoiceTrials: FC<{ marginBottom: theme.spacing(4), }} > - - {formatDate(choice.timestamp)} - + + {formatDate(choice.timestamp)} + + + {enabled ? : } + + None: storage = optuna.storages.InMemoryStorage() - study = create_study(storage=storage) + study = create_study(storage=storage, n_generate=3) for _ in range(3): - trial = study.ask() - study.mark_comparison_ready(trial) + study.ask() app = create_app(storage) study_id = study._study._study_id @@ -227,14 +226,14 @@ class APITestCase(TestCase): content_type="application/json", ) self.assertEqual(status, 204) - histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 assert histories[0]["enabled"] - history_uuid = histories[0]["uuid"] + history_id = histories[0]["id"] status, _, _ = send_request( app, - f"/api/studies/{study_id}/preference/{history_uuid}", + f"/api/studies/{study_id}/preference/{history_id}", "PUT", body=json.dumps( { @@ -244,14 +243,14 @@ class APITestCase(TestCase): content_type="application/json", ) self.assertEqual(status, 204) - histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 assert not histories[0]["enabled"] assert len(study.get_preferences()) == 0 status, _, _ = send_request( app, - f"/api/studies/{study_id}/preference/{history_uuid}", + f"/api/studies/{study_id}/preference/{history_id}", "PUT", body=json.dumps( { @@ -261,7 +260,7 @@ class APITestCase(TestCase): content_type="application/json", ) self.assertEqual(status, 204) - histories = serialize_preference_histories(storage.get_study_system_attrs(study_id)) + histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 assert histories[0]["enabled"] preferences = study.get_preferences() diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index eb5ffbce..e1e18b6e 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -1,9 +1,13 @@ from __future__ import annotations +import json from typing import Callable +from typing import TYPE_CHECKING +from optuna_dashboard._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from optuna_dashboard._preferential_history import NewHistory from optuna_dashboard._preferential_history import report_history +from optuna_dashboard._preferential_history import switching_history from optuna_dashboard._serializer import serialize_preference_history from optuna_dashboard.preferential import create_study from optuna_dashboard.preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE @@ -12,6 +16,10 @@ from .storage_supplier import parametrize_storages from .storage_supplier import StorageSupplier +if TYPE_CHECKING: + from optuna_dashboard._preferential_history import History + + @parametrize_storages def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: @@ -25,20 +33,12 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) report_history( study_id=study_id, storage=storage, - input_data=NewHistory( - mode="ChooseWorst", - candidates=[0, 1, 2], - clicked=1, - ), + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), ) report_history( study_id=study_id, storage=storage, - input_data=NewHistory( - mode="ChooseWorst", - candidates=[0, 2, 3, 4], - clicked=0, - ), + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 2, 3, 4], clicked=0), ) history = serialize_preference_history(storage.get_study_system_attrs(study_id)) sys_attrs = storage.get_study_system_attrs(study_id) @@ -61,56 +61,53 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert preferences[i][1] == worst -# TODO(moririn): Add tests for switching_history. -# @parametrize_storages -# def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> None: -# with storage_supplier() as storage: -# study = create_study(storage=storage, n_generate=5) -# for _ in range(5): -# trial = study.ask() -# trial.suggest_float("x", 0, 1) +@parametrize_storages +def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> None: + with storage_supplier() as storage: + study = create_study(storage=storage, n_generate=5) + for _ in range(5): + trial = study.ask() + trial.suggest_float("x", 0, 1) -# study_id = study._study._study_id + study_id = study._study._study_id -# history_uuid = report_history( -# study_id=study_id, -# storage=storage, -# input_data={ -# "mode": "ChooseWorst", -# "candidates": [0, 1, 2], -# "clicked": 1, -# }, -# ) -# switching_history(study_id, storage, history_uuid, False) -# history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) -# preference = get_preference(study_id, storage, history.preference_uuid) -# assert history.mode == "ChooseWorst" -# assert history.candidates == [0, 1, 2] -# assert history.clicked == 1 -# assert len(history.evacuated_preference) == 2 -# assert len(preference) == 0 + def get_preferences_history(history_id: str): + system_attrs = storage.get_study_system_attrs(study_id) + history: History = json.loads( + system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + history_id, "") + ) + preference: list[tuple[int, int]] = system_attrs.get( + _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], [] + ) + return preference, history -# switching_history(study_id, storage, history_uuid, False) -# history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) -# preference = get_preference(study_id, storage, history.preference_uuid) -# assert len(history.evacuated_preference) == 2 -# assert len(preference) == 0 + history_id = report_history( + study_id=study_id, + storage=storage, + input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), + ) + switching_history(study_id, storage, history_id, False) + preference, history = get_preferences_history(history_id) + assert history["mode"] == "ChooseWorst" + assert history["candidates"] == [0, 1, 2] + assert history["clicked"] == 1 + assert len(preference) == 0 -# switching_history(study_id, storage, history_uuid, True) -# history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) -# preference = get_preference(study_id, storage, history.preference_uuid) -# assert history.mode == "ChooseWorst" -# assert history.candidates == [0, 1, 2] -# assert history.clicked == 1 -# assert len(history.evacuated_preference) == 0 -# assert len(preference) == 2 -# for i, (best, worst) in enumerate([(0, 1), (2, 1)]): -# assert len(preference[i]) == 2 -# assert preference[i][0] == best -# assert preference[i][1] == worst + switching_history(study_id, storage, history_id, False) + preference, history = get_preferences_history(history_id) + assert len(preference) == 0 -# switching_history(study_id, storage, history_uuid, True) -# history = load_preference_history(history_uuid, storage.get_study_system_attrs(study_id)) -# preference = get_preference(study_id, storage, history.preference_uuid) -# assert len(history.evacuated_preference) == 0 -# assert len(preference) == 2 + switching_history(study_id, storage, history_id, True) + preference, history = get_preferences_history(history_id) + assert history["mode"] == "ChooseWorst" + assert history["candidates"] == [0, 1, 2] + assert history["clicked"] == 1 + assert len(preference) == 2 + for i, (best, worst) in enumerate([(0, 1), (2, 1)]): + assert len(preference[i]) == 2 + assert preference[i][0] == best + assert preference[i][1] == worst + + switching_history(study_id, storage, history_id, True) + preference, history = get_preferences_history(history_id) + assert len(preference) == 2 From 2f1cb918862daccbcf2b7b835dfe874f4283f7f8 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 8 Sep 2023 16:57:11 +0900 Subject: [PATCH 04/22] add undo button on feedback screen --- .../ts/components/PreferenceHistory.tsx | 8 +-- .../ts/components/PreferentialTrials.tsx | 49 +++++++++++++++---- 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index bfacfc62..0535dc2e 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -145,16 +145,17 @@ const ChoiceTrials: FC<{ const [enabled, setEnabled] = useState(choice.enabled) const theme = useTheme() const worst_trials = new Set([choice.clicked]) - const actions = actionCreator() + const action = actionCreator() const handleSwitch = () => { setEnabled(!enabled) - actions.switchPreferentialHistory(study_id, choice.id, !enabled) + action.switchPreferentialHistory(study_id, choice.id, !enabled) } return ( - {enabled ? : } + {choice.enabled ? : } {choice.candidates.map((trial_num, index) => ( diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 08efd9a7..ab7f04ea 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -14,6 +14,7 @@ import OpenInFullIcon from "@mui/icons-material/OpenInFull" import ReplayIcon from "@mui/icons-material/Replay" import Modal from "@mui/material/Modal" import { red } from "@mui/material/colors" +import UndoIcon from "@mui/icons-material/Undo" import { actionCreator } from "../action" import { TrialListDetail } from "./TrialList" @@ -180,10 +181,13 @@ type DisplayTrials = { export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ studyDetail, }) => { + const [undoHistoryId, setUndoHistoryId] = useState(null) + if (studyDetail === null || !studyDetail.is_preferential) { return null } const theme = useTheme() + const action = actionCreator() const runningTrials = studyDetail.trials.filter((t) => t.state === "Running") const activeTrials = runningTrials.concat(studyDetail.best_trials) @@ -229,18 +233,45 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ } }) } + const latestHistoryId = studyDetail?.preference_history + ?.filter((h) => h.enabled) + .pop()?.id + if (undoHistoryId !== null && undoHistoryId !== latestHistoryId) { + setUndoHistoryId(null) + } return ( - - Which trial is the worst? - + + + Which trial is the worst? + + { + if (latestHistoryId === undefined) { + return + } + setUndoHistoryId(latestHistoryId) + action.switchPreferentialHistory( + studyDetail.id, + latestHistoryId, + false + ) + }} + sx={{ + margin: "auto 0 auto auto", + }} + > + + + {displayTrials.numbers.map((t, index) => ( Date: Fri, 8 Sep 2023 17:03:45 +0900 Subject: [PATCH 05/22] fix by lint --- optuna_dashboard/_serializer.py | 6 +++--- python_tests/test_preferential_history.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index bcbe1ddf..5ac9c55b 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -173,14 +173,14 @@ def serialize_study_detail( def serialize_preference_history( system_attrs: dict[str, Any], -) -> list[History]: - histories: list[History] = [] +) -> list[dict[str, Any]]: + histories: list[dict[str, Any]] = [] for k, v in system_attrs.items(): if not k.startswith(_SYSTEM_ATTR_PREFIX_HISTORY): continue choice: dict[str, Any] = json.loads(v) if choice["mode"] == "ChooseWorst": - history: ChooseWorstHistory = { + history = { "mode": "ChooseWorst", "id": choice["id"], "preference_id": choice["preference_id"], diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index e1e18b6e..75e622db 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -71,7 +71,7 @@ def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> N study_id = study._study._study_id - def get_preferences_history(history_id: str): + def get_preferences_history(history_id: str) -> tuple[list[tuple[int, int]], History]: system_attrs = storage.get_study_system_attrs(study_id) history: History = json.loads( system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + history_id, "") From 23db3e7c4ecd4bade3851bee474ffe1991dd556b Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 8 Sep 2023 17:09:43 +0900 Subject: [PATCH 06/22] fix by lint --- optuna_dashboard/_serializer.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 5ac9c55b..35b9d6a7 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -25,9 +25,6 @@ if TYPE_CHECKING: from typing import Literal from typing import TypedDict - from ._preferential_history import ChooseWorstHistory - from ._preferential_history import History - Attribute = TypedDict( "Attribute", { From b0ed82af7e779d4235e80babde6d0c28bc41c9b0 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Mon, 11 Sep 2023 16:31:39 +0900 Subject: [PATCH 07/22] fix by review --- optuna_dashboard/_app.py | 22 +++++++------- optuna_dashboard/_preferential_history.py | 29 ++++++++++--------- optuna_dashboard/_serializer.py | 4 +-- .../preferential/_system_attrs.py | 4 +-- python_tests/test_api.py | 14 ++------- python_tests/test_preferential_history.py | 11 +++---- 6 files changed, 38 insertions(+), 46 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 65bb9f34..f497145e 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -29,8 +29,9 @@ from ._custom_plot_data import get_plotly_graph_objects from ._importance import get_param_importance_from_trials_cache from ._pareto_front import get_pareto_front_trials from ._preferential_history import NewHistory +from ._preferential_history import remove_history from ._preferential_history import report_history -from ._preferential_history import switching_history +from ._preferential_history import restore_history from ._rdb_migration import register_rdb_migration_route from ._serializer import serialize_study_detail from ._serializer import serialize_study_summary @@ -307,18 +308,17 @@ def create_app( response.status = 204 return {} - @app.put("/api/studies//preference/") + @app.delete("/api/studies//preference/") @json_api_view - def switch_preference(study_id: int, history_uuid: str) -> dict[str, Any]: - try: - enable = request.json.get("enable", None) - if enable is None or not isinstance(enable, bool): - raise ValueError - except ValueError: - response.status = 400 - return {"reason": "Invalid request."} - switching_history(study_id, storage, history_uuid, enable) + def remove_preference(study_id: int, history_uuid: str) -> dict[str, Any]: + remove_history(study_id, storage, history_uuid) + response.status = 204 + return {} + @app.post("/api/studies//preference/") + @json_api_view + def restore_preference(study_id: int, history_uuid: str) -> dict[str, Any]: + restore_history(study_id, storage, history_uuid) response.status = 204 return {} diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index a8fe2148..60fe6cb0 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -82,19 +82,20 @@ def report_history( return history_id -def switching_history(study_id: int, storage: BaseStorage, uuid: str, enable: bool) -> None: +def remove_history(study_id: int, storage: BaseStorage, uuid: str) -> None: system_attrs = storage.get_study_system_attrs(study_id) history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, "")) - if enable: - preferences = [ - (best, history["clicked"]) - for best in history["candidates"] - if best != history["clicked"] - ] - storage.set_study_system_attr( - study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], preferences - ) - else: # disable - storage.set_study_system_attr( - study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], [] - ) + storage.set_study_system_attr( + study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], [] + ) + + +def restore_history(study_id: int, storage: BaseStorage, uuid: str) -> None: + system_attrs = storage.get_study_system_attrs(study_id) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, "")) + preferences = [ + (best, history["clicked"]) for best in history["candidates"] if best != history["clicked"] + ] + storage.set_study_system_attr( + study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], preferences + ) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 35b9d6a7..9ce77fb3 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -18,7 +18,7 @@ from ._named_objectives import get_objective_names from ._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from .artifact._backend import list_trial_artifacts from .preferential._study import _SYSTEM_ATTR_PREFERENTIAL_STUDY -from .preferential._system_attrs import is_preference_valid +from .preferential._system_attrs import is_preference_removed if TYPE_CHECKING: @@ -184,7 +184,7 @@ def serialize_preference_history( "timestamp": choice["timestamp"], "candidates": choice["candidates"], "clicked": choice["clicked"], - "enabled": is_preference_valid(system_attrs, choice["preference_id"]), + "is_removed": is_preference_removed(system_attrs, choice["preference_id"]), } histories.append(history) diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 5ce83742..82ba61fa 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -44,10 +44,10 @@ def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]] return preferences -def is_preference_valid(study_system_attrs: dict[str, Any], uuid: str) -> bool: +def is_preference_removed(study_system_attrs: dict[str, Any], uuid: str) -> bool: key = _SYSTEM_ATTR_PREFIX_PREFERENCE + uuid preference = study_system_attrs.get(key, []) - return len(preference) > 0 + return len(preference) == 0 def report_skip( diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 22c60578..205bf966 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -234,12 +234,7 @@ class APITestCase(TestCase): status, _, _ = send_request( app, f"/api/studies/{study_id}/preference/{history_id}", - "PUT", - body=json.dumps( - { - "enable": False, - } - ), + "DELETE", content_type="application/json", ) self.assertEqual(status, 204) @@ -251,12 +246,7 @@ class APITestCase(TestCase): status, _, _ = send_request( app, f"/api/studies/{study_id}/preference/{history_id}", - "PUT", - body=json.dumps( - { - "enable": True, - } - ), + "POST", content_type="application/json", ) self.assertEqual(status, 204) diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index 75e622db..3ba54fd4 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -6,8 +6,9 @@ from typing import TYPE_CHECKING from optuna_dashboard._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from optuna_dashboard._preferential_history import NewHistory +from optuna_dashboard._preferential_history import remove_history from optuna_dashboard._preferential_history import report_history -from optuna_dashboard._preferential_history import switching_history +from optuna_dashboard._preferential_history import restore_history from optuna_dashboard._serializer import serialize_preference_history from optuna_dashboard.preferential import create_study from optuna_dashboard.preferential._system_attrs import _SYSTEM_ATTR_PREFIX_PREFERENCE @@ -86,18 +87,18 @@ def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> N storage=storage, input_data=NewHistory(mode="ChooseWorst", candidates=[0, 1, 2], clicked=1), ) - switching_history(study_id, storage, history_id, False) + remove_history(study_id, storage, history_id) preference, history = get_preferences_history(history_id) assert history["mode"] == "ChooseWorst" assert history["candidates"] == [0, 1, 2] assert history["clicked"] == 1 assert len(preference) == 0 - switching_history(study_id, storage, history_id, False) + remove_history(study_id, storage, history_id) preference, history = get_preferences_history(history_id) assert len(preference) == 0 - switching_history(study_id, storage, history_id, True) + restore_history(study_id, storage, history_id) preference, history = get_preferences_history(history_id) assert history["mode"] == "ChooseWorst" assert history["candidates"] == [0, 1, 2] @@ -108,6 +109,6 @@ def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> N assert preference[i][0] == best assert preference[i][1] == worst - switching_history(study_id, storage, history_id, True) + restore_history(study_id, storage, history_id) preference, history = get_preferences_history(history_id) assert len(preference) == 2 From f9aa926a97e1f1481cc36f8169bd0f862b9c1106 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 11:41:18 +0900 Subject: [PATCH 08/22] fix by review --- optuna_dashboard/_app.py | 12 ++-- optuna_dashboard/_preferential_history.py | 40 +++++------ optuna_dashboard/_serializer.py | 3 +- .../preferential/_system_attrs.py | 4 +- optuna_dashboard/ts/action.ts | 23 ++++--- optuna_dashboard/ts/apiClient.ts | 23 ++++--- .../ts/components/PreferenceHistory.tsx | 44 ++++++++----- .../ts/components/PreferentialTrials.tsx | 66 +++++++++---------- optuna_dashboard/ts/types/index.d.ts | 3 +- python_tests/test_api.py | 6 +- python_tests/test_preferential_history.py | 12 ++-- 11 files changed, 125 insertions(+), 111 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index f497145e..a3ab595a 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -308,17 +308,17 @@ def create_app( response.status = 204 return {} - @app.delete("/api/studies//preference/") + @app.delete("/api/studies//preference/") @json_api_view - def remove_preference(study_id: int, history_uuid: str) -> dict[str, Any]: - remove_history(study_id, storage, history_uuid) + def remove_preference(study_id: int, history_id: str) -> dict[str, Any]: + remove_history(study_id, storage, history_id) response.status = 204 return {} - @app.post("/api/studies//preference/") + @app.post("/api/studies//preference/") @json_api_view - def restore_preference(study_id: int, history_uuid: str) -> dict[str, Any]: - restore_history(study_id, storage, history_uuid) + def restore_preference(study_id: int, history_id: str) -> dict[str, Any]: + restore_history(study_id, storage, history_id) response.status = 204 return {} diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index 60fe6cb0..c8acf969 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -24,10 +24,10 @@ if TYPE_CHECKING: { "mode": FeedbackMode, "id": str, - "preference_id": str, "timestamp": str, "candidates": list[int], "clicked": int, + "preferences": list[tuple[int, int]], }, ) History = ChooseWorstHistory @@ -49,53 +49,45 @@ def report_history( # TODO(moririn): Use TypeGuard after adding other history types. if input_data.mode == "ChooseWorst": preferences = [ - (best, input_data.clicked) - for best in input_data.candidates - if best != input_data.clicked + (better, input_data.clicked) + for better in input_data.candidates + if better != input_data.clicked ] else: assert False, f"Unknown data: {input_data}" - preference_id = report_preferences( + id = report_preferences( study_id=study_id, storage=storage, preferences=preferences, ) - history_id = str(uuid.uuid4()) if input_data.mode == "ChooseWorst": history: ChooseWorstHistory = { "mode": "ChooseWorst", - "id": history_id, - "preference_id": preference_id, + "id": id, "timestamp": datetime.now().isoformat(), "candidates": input_data.candidates, "clicked": input_data.clicked, + "preferences": preferences, } - key = _SYSTEM_ATTR_PREFIX_HISTORY + history_id + key = _SYSTEM_ATTR_PREFIX_HISTORY + id storage.set_study_system_attr( study_id=study_id, key=key, value=json.dumps(history), ) - return history_id + return id -def remove_history(study_id: int, storage: BaseStorage, uuid: str) -> None: +def remove_history(study_id: int, storage: BaseStorage, id: str) -> None: + storage.set_study_system_attr(study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + id, []) + + +def restore_history(study_id: int, storage: BaseStorage, id: str) -> None: system_attrs = storage.get_study_system_attrs(study_id) - history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, "")) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) storage.set_study_system_attr( - study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], [] - ) - - -def restore_history(study_id: int, storage: BaseStorage, uuid: str) -> None: - system_attrs = storage.get_study_system_attrs(study_id) - history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + uuid, "")) - preferences = [ - (best, history["clicked"]) for best in history["candidates"] if best != history["clicked"] - ] - storage.set_study_system_attr( - study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], preferences + study_id, _SYSTEM_ATTR_PREFIX_PREFERENCE + history["id"], history["preferences"] ) diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 9ce77fb3..d4ff5608 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -180,11 +180,10 @@ def serialize_preference_history( history = { "mode": "ChooseWorst", "id": choice["id"], - "preference_id": choice["preference_id"], "timestamp": choice["timestamp"], "candidates": choice["candidates"], "clicked": choice["clicked"], - "is_removed": is_preference_removed(system_attrs, choice["preference_id"]), + "is_removed": is_preference_removed(system_attrs, choice["id"]), } histories.append(history) diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 82ba61fa..33d56f30 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -44,8 +44,8 @@ def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]] return preferences -def is_preference_removed(study_system_attrs: dict[str, Any], uuid: str) -> bool: - key = _SYSTEM_ATTR_PREFIX_PREFERENCE + uuid +def is_preference_removed(study_system_attrs: dict[str, Any], id: str) -> bool: + key = _SYSTEM_ATTR_PREFIX_PREFERENCE + id preference = study_system_attrs.get(key, []) return len(preference) == 0 diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index f42a17bb..5b66c58f 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -16,7 +16,8 @@ import { deleteArtifactAPI, reportPreferenceAPI, skipPreferentialTrialAPI, - switchPreferentialHistoryAPI, + removePreferentialHistoryAPI, + restorePreferentialHistoryAPI, } from "./apiClient" import { graphVisibilityState, @@ -610,12 +611,17 @@ export const actionCreator = () => { }) } - const switchPreferentialHistory = ( - studyId: number, - historyUuid: string, - enable: boolean - ) => { - switchPreferentialHistoryAPI(studyId, historyUuid, enable).catch((err) => { + const removePreferentialHistory = (studyId: number, historyUuid: string) => { + removePreferentialHistoryAPI(studyId, historyUuid).catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { + variant: "error", + }) + console.log(err) + }) + } + const restorePreferentialHistory = (studyId: number, historyUuid: string) => { + restorePreferentialHistoryAPI(studyId, historyUuid).catch((err) => { const reason = err.response?.data.reason enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { variant: "error", @@ -645,7 +651,8 @@ export const actionCreator = () => { saveTrialUserAttrs, updatePreference, skipPreferentialTrial, - switchPreferentialHistory, + removePreferentialHistory, + restorePreferentialHistory, } } diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index 5c9355b7..ecc4b7c2 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -57,12 +57,11 @@ const convertTrialResponse = (res: TrialResponse): Trial => { interface PreferenceHistoryResponce { id: string - preference_id: string candidates: number[] clicked: number mode: PreferenceFeedbackMode timestamp: string - enabled: boolean + is_removed: boolean } const convertPreferenceHistory = ( @@ -70,12 +69,11 @@ const convertPreferenceHistory = ( ): PreferenceHistory => { return { id: res.id, - preference_id: res.preference_id, candidates: res.candidates, clicked: res.clicked, feedback_mode: res.mode, timestamp: new Date(res.timestamp), - enabled: res.enabled, + isRemoved: res.is_removed, } } @@ -369,15 +367,22 @@ export const skipPreferentialTrialAPI = ( }) } -export const switchPreferentialHistoryAPI = ( +export const removePreferentialHistoryAPI = ( studyId: number, - historyUuid: string, - enable: boolean + historyUuid: string ): Promise => { return axiosInstance - .put(`/api/studies/${studyId}/preference/${historyUuid}`, { - enable: enable, + .delete(`/api/studies/${studyId}/preference/${historyUuid}`) + .then(() => { + return }) +} +export const restorePreferentialHistoryAPI = ( + studyId: number, + historyUuid: string +): Promise => { + return axiosInstance + .post(`/api/studies/${studyId}/preference/${historyUuid}`) .then(() => { return }) diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index 0535dc2e..81f106bd 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -142,14 +142,10 @@ const ChoiceTrials: FC<{ trials: Trial[] study_id: number }> = ({ choice, trials, study_id }) => { - const [enabled, setEnabled] = useState(choice.enabled) + const [isRemoved, setRemoved] = useState(choice.isRemoved) const theme = useTheme() const worst_trials = new Set([choice.clicked]) const action = actionCreator() - const handleSwitch = () => { - setEnabled(!enabled) - action.switchPreferentialHistory(study_id, choice.id, !enabled) - } return ( {formatDate(choice.timestamp)} - - {choice.enabled ? : } - + {choice.isRemoved ? ( + { + setRemoved(false) + action.restorePreferentialHistory(study_id, choice.id) + }} + sx={{ + margin: `auto ${theme.spacing(2)}`, + }} + > + + + ) : ( + { + setRemoved(true) + action.removePreferentialHistory(study_id, choice.id) + }} + sx={{ + margin: `auto ${theme.spacing(2)}`, + }} + > + + + )} {choice.candidates.map((trial_num, index) => ( diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index ab7f04ea..f079a80d 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -174,8 +174,8 @@ const PreferentialTrial: FC<{ } type DisplayTrials = { - numbers: number[] - last_number: number + display: number[] + clicked: number[] } export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ @@ -193,51 +193,55 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ const activeTrials = runningTrials.concat(studyDetail.best_trials) const [displayTrials, setDisplayTrials] = useState({ - numbers: activeTrials.map((t) => t.number), - last_number: Math.max(...activeTrials.map((t) => t.number), -1), + display: [], + clicked: [], }) const new_trails = activeTrials.filter( (t) => - displayTrials.last_number < t.number && - displayTrials.numbers.find((n) => n === t.number) === undefined + !displayTrials.display.includes(t.number) && + !displayTrials.clicked.includes(t.number) ) if (new_trails.length > 0) { - setDisplayTrials((display) => { - const numbers = [...display.numbers] + setDisplayTrials((prev) => { + const display = [...prev.display] + const clicked = [...prev.clicked] new_trails.map((t) => { - const index = numbers.findIndex((n) => n === -1) + const index = display.findIndex((n) => n === -1) if (index === -1) { - numbers.push(t.number) + display.push(t.number) + clicked.push(-1) } else { - numbers[index] = t.number + display[index] = t.number } }) return { - numbers: numbers, - last_number: Math.max(...numbers, -1), + display: display, + clicked: clicked, } }) } const hideTrial = (num: number) => { - setDisplayTrials((display) => { - const index = display.numbers.findIndex((n) => n === num) + setDisplayTrials((prev) => { + const index = prev.display.findIndex((n) => n === num) if (index === -1) { - return display + return prev } - const numbers = [...displayTrials.numbers] - numbers[index] = -1 + const display = [...prev.display] + const clicked = [...prev.clicked] + display[index] = -1 + clicked[index] = num return { - numbers: numbers, - last_number: display.last_number, + display: display, + clicked: clicked, } }) } - const latestHistoryId = studyDetail?.preference_history - ?.filter((h) => h.enabled) - .pop()?.id + const latestHistoryId = + studyDetail?.preference_history?.filter((h) => !h.isRemoved).pop()?.id ?? + null if (undoHistoryId !== null && undoHistoryId !== latestHistoryId) { - setUndoHistoryId(null) + setUndoHistoryId(latestHistoryId) } return ( @@ -253,17 +257,13 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ Which trial is the worst? { - if (latestHistoryId === undefined) { + if (latestHistoryId === null) { return } setUndoHistoryId(latestHistoryId) - action.switchPreferentialHistory( - studyDetail.id, - latestHistoryId, - false - ) + action.removePreferentialHistory(studyDetail.id, latestHistoryId) }} sx={{ margin: "auto 0 auto auto", @@ -273,11 +273,11 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ - {displayTrials.numbers.map((t, index) => ( + {displayTrials.display.map((t, index) => ( trial.number === t)} - candidates={displayTrials.numbers.filter((n) => n !== -1)} + candidates={displayTrials.display.filter((n) => n !== -1)} hideTrial={() => { hideTrial(t) }} diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index fd9ca113..014d2dfd 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -217,10 +217,9 @@ type StudyParamImportance = { type PreferenceHistory = { id: string - preference_id: string candidates: number[] clicked: number feedback_mode: PreferenceFeedbackMode timestamp: Date - enabled: boolean + isRemoved: boolean } diff --git a/python_tests/test_api.py b/python_tests/test_api.py index 205bf966..6075ffa5 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -228,7 +228,7 @@ class APITestCase(TestCase): self.assertEqual(status, 204) histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 - assert histories[0]["enabled"] + assert not histories[0]["is_removed"] history_id = histories[0]["id"] status, _, _ = send_request( @@ -240,7 +240,7 @@ class APITestCase(TestCase): self.assertEqual(status, 204) histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 - assert not histories[0]["enabled"] + assert histories[0]["is_removed"] assert len(study.get_preferences()) == 0 status, _, _ = send_request( @@ -252,7 +252,7 @@ class APITestCase(TestCase): self.assertEqual(status, 204) histories = serialize_preference_history(storage.get_study_system_attrs(study_id)) assert len(histories) == 1 - assert histories[0]["enabled"] + assert not histories[0]["is_removed"] preferences = study.get_preferences() preferences.sort(key=lambda x: (x[0].number, x[1].number)) assert len(preferences) == 2 diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index 3ba54fd4..c64f448c 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -46,7 +46,7 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert len(history) == 2 assert history[0]["candidates"] == [0, 1, 2] assert history[0]["clicked"] == 1 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["preference_id"]] + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[0]["id"]] assert len(preferences) == 2 for i, (best, worst) in enumerate([(0, 1), (2, 1)]): assert len(preferences[i]) == 2 @@ -54,7 +54,7 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) assert preferences[i][1] == worst assert history[1]["candidates"] == [0, 2, 3, 4] assert history[1]["clicked"] == 0 - preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["preference_id"]] + preferences = sys_attrs[_SYSTEM_ATTR_PREFIX_PREFERENCE + history[1]["id"]] assert len(preferences) == 3 for i, (best, worst) in enumerate([(2, 0), (3, 0), (4, 0)]): assert len(preferences[i]) == 2 @@ -72,13 +72,11 @@ def test_undo_redo_history(storage_supplier: Callable[[], StorageSupplier]) -> N study_id = study._study._study_id - def get_preferences_history(history_id: str) -> tuple[list[tuple[int, int]], History]: + def get_preferences_history(id: str) -> tuple[list[tuple[int, int]], History]: system_attrs = storage.get_study_system_attrs(study_id) - history: History = json.loads( - system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + history_id, "") - ) + history: History = json.loads(system_attrs.get(_SYSTEM_ATTR_PREFIX_HISTORY + id, "")) preference: list[tuple[int, int]] = system_attrs.get( - _SYSTEM_ATTR_PREFIX_PREFERENCE + history["preference_id"], [] + _SYSTEM_ATTR_PREFIX_PREFERENCE + id, [] ) return preference, history From ed054fb9ae0f4dd7b6a350b46820136df311919b Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 13:37:02 +0900 Subject: [PATCH 09/22] fix feedback screen --- optuna_dashboard/ts/components/PreferentialTrials.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index f079a80d..d4a06c54 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -241,7 +241,7 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ studyDetail?.preference_history?.filter((h) => !h.isRemoved).pop()?.id ?? null if (undoHistoryId !== null && undoHistoryId !== latestHistoryId) { - setUndoHistoryId(latestHistoryId) + setUndoHistoryId(null) } return ( From bc4a76ea1f4f87565ebf1b4666a5f9e0e22d84b7 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 14:48:48 +0900 Subject: [PATCH 10/22] fix ui by review --- optuna_dashboard/ts/components/PreferenceHistory.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index 81f106bd..9e19bf07 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -204,6 +204,7 @@ const ChoiceTrials: FC<{ flexDirection: "row", flexWrap: "wrap", filter: choice.isRemoved ? "brightness(0.4)" : undefined, + backgroundColor: theme.palette.background.paper, }} > {choice.candidates.map((trial_num, index) => ( From d88525a1bc7e0bf6cb00264ed1cece449fb4f566 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Tue, 12 Sep 2023 14:49:04 +0900 Subject: [PATCH 11/22] fix by lint --- optuna_dashboard/_preferential_history.py | 1 - optuna_dashboard/_serializer.py | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/optuna_dashboard/_preferential_history.py b/optuna_dashboard/_preferential_history.py index c8acf969..3d912190 100644 --- a/optuna_dashboard/_preferential_history.py +++ b/optuna_dashboard/_preferential_history.py @@ -4,7 +4,6 @@ from dataclasses import dataclass from datetime import datetime import json from typing import TYPE_CHECKING -import uuid from optuna.storages import BaseStorage diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index 17c9518f..8388c736 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -18,8 +18,8 @@ from ._named_objectives import get_objective_names from ._preferential_history import _SYSTEM_ATTR_PREFIX_HISTORY from .artifact._backend import list_trial_artifacts from .preferential._study import _SYSTEM_ATTR_PREFERENTIAL_STUDY -from .preferential._system_attrs import is_preference_removed from .preferential._system_attrs import get_preferences +from .preferential._system_attrs import is_preference_removed if TYPE_CHECKING: From 4d15b79e9b414a68e1f6d5f65e2828e54e35894f Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Thu, 14 Sep 2023 15:20:46 +0900 Subject: [PATCH 12/22] fixed by merge --- optuna_dashboard/ts/components/PreferenceHistory.tsx | 6 +++--- optuna_dashboard/ts/components/PreferentialTrials.tsx | 5 +++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index 9e19bf07..db4df078 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -142,7 +142,7 @@ const ChoiceTrials: FC<{ trials: Trial[] study_id: number }> = ({ choice, trials, study_id }) => { - const [isRemoved, setRemoved] = useState(choice.isRemoved) + const [isRemoved, setRemoved] = useState(choice.is_removed) const theme = useTheme() const worst_trials = new Set([choice.clicked]) const action = actionCreator() @@ -170,7 +170,7 @@ const ChoiceTrials: FC<{ > {formatDate(choice.timestamp)} - {choice.isRemoved ? ( + {choice.is_removed ? ( { @@ -203,7 +203,7 @@ const ChoiceTrials: FC<{ display: "flex", flexDirection: "row", flexWrap: "wrap", - filter: choice.isRemoved ? "brightness(0.4)" : undefined, + filter: choice.is_removed ? "brightness(0.4)" : undefined, backgroundColor: theme.palette.background.paper, }} > diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 7fcb634f..26cc4976 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -196,7 +196,8 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ const hiddenTrials = new Set( studyDetail.preference_history - ?.map((p) => p.clicked) + ?.filter((h) => !h.is_removed) + .map((p) => p.clicked) .concat(studyDetail.skipped_trials) ?? [] ) const activeTrials = studyDetail.trials.filter( @@ -256,7 +257,7 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ }) } const latestHistoryId = - studyDetail?.preference_history?.filter((h) => !h.isRemoved).pop()?.id ?? + studyDetail?.preference_history?.filter((h) => !h.is_removed).pop()?.id ?? null if (undoHistoryId !== null && undoHistoryId !== latestHistoryId) { setUndoHistoryId(null) From eb6234eabc683e82c9c09379d938cdb400a631c1 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Thu, 14 Sep 2023 17:39:06 +0900 Subject: [PATCH 13/22] erase isolated node from graph --- optuna_dashboard/ts/components/PreferentialGraph.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferentialGraph.tsx b/optuna_dashboard/ts/components/PreferentialGraph.tsx index 48364598..f343cb06 100644 --- a/optuna_dashboard/ts/components/PreferentialGraph.tsx +++ b/optuna_dashboard/ts/components/PreferentialGraph.tsx @@ -181,6 +181,7 @@ export const PreferentialGraph: FC<{ if (!studyDetail.is_preferential || studyDetail.preferences === undefined) return const preferences = reductionPreference(studyDetail.preferences) + const trialNodes = Array.from(new Set(preferences.flat())) const graph: ElkNode = { id: "root", layoutOptions: { @@ -189,8 +190,8 @@ export const PreferentialGraph: FC<{ "elk.layered.spacing.nodeNodeBetweenLayers": nodeMargin.toString(), "elk.spacing.nodeNode": nodeMargin.toString(), }, - children: studyDetail.trials.map((trial) => ({ - id: `${trial.number}`, + children: trialNodes.map((trial) => ({ + id: `${trial}`, targetPosition: "top", sourcePosition: "bottom", width: nodeWidth, @@ -207,7 +208,7 @@ export const PreferentialGraph: FC<{ .then((layoutedGraph) => { setNodes( layoutedGraph.children?.map((node, index) => { - const trial = studyDetail.trials[index] + const trial = studyDetail.trials[trialNodes[index]] return { id: `${trial.number}`, type: "note", From 29fa96d3eb9f0cc5d7d8cd2c0df3c01bc8a98acd Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 15 Sep 2023 14:56:42 +0900 Subject: [PATCH 14/22] fix by review --- optuna_dashboard/_app.py | 6 ++---- optuna_dashboard/_serializer.py | 2 +- optuna_dashboard/ts/apiClient.ts | 4 ++-- optuna_dashboard/ts/components/PreferentialTrials.tsx | 2 +- optuna_dashboard/ts/types/index.d.ts | 2 +- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 731de524..87efaae5 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -221,10 +221,8 @@ def create_app( ) = get_cached_extra_study_property(study_id, trials) plotly_graph_objects = get_plotly_graph_objects(system_attrs) - trials_id2number = {trial._trial_id: trial.number for trial in trials} - skipped_trials = [ - trials_id2number[trial_id] for trial_id in get_skipped_trial_ids(system_attrs) - ] + skipped_trial_ids = get_skipped_trial_ids(system_attrs) + skipped_trials = [t.number for t in trials if t._trial_id in skipped_trial_ids] return serialize_study_detail( summary, best_trials, diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index d3cd786f..af3ed242 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -167,7 +167,7 @@ def serialize_study_detail( if serialized["is_preferential"]: serialized["preference_history"] = serialize_preference_history(system_attrs) serialized["preferences"] = get_preferences(system_attrs) - serialized["skipped_trials"] = skipped_trials + serialized["skipped_trial_numbers"] = skipped_trials serialized["plotly_graph_objects"] = [ {"id": id_, "graph_object": graph_object} for id_, graph_object in plotly_graph_objects.items() diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index abfe057a..9f9cd8cb 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -99,7 +99,7 @@ interface StudyDetailResponse { preferences?: [number, number][] preference_history?: PreferenceHistoryResponce[] plotly_graph_objects: PlotlyGraphObject[] - skipped_trials?: number[] + skipped_trial_numbers?: number[] } export const getStudyDetailAPI = ( @@ -140,7 +140,7 @@ export const getStudyDetailAPI = ( convertPreferenceHistory ), plotly_graph_objects: res.data.plotly_graph_objects, - skipped_trials: res.data.skipped_trials ?? [], + skipped_trial_numbers: res.data.skipped_trial_numbers ?? [], } }) } diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 26cc4976..5fc0ac32 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -198,7 +198,7 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ studyDetail.preference_history ?.filter((h) => !h.is_removed) .map((p) => p.clicked) - .concat(studyDetail.skipped_trials) ?? [] + .concat(studyDetail.skipped_trial_numbers) ?? [] ) const activeTrials = studyDetail.trials.filter( (t) => diff --git a/optuna_dashboard/ts/types/index.d.ts b/optuna_dashboard/ts/types/index.d.ts index 01868f7b..a51a88d9 100644 --- a/optuna_dashboard/ts/types/index.d.ts +++ b/optuna_dashboard/ts/types/index.d.ts @@ -206,7 +206,7 @@ type StudyDetail = { preferences?: [number, number][] preference_history?: PreferenceHistory[] plotly_graph_objects: PlotlyGraphObject[] - skipped_trials: number[] + skipped_trial_numbers: number[] } type StudyDetails = { From aec4aa03208c38dc193d63eba0e0c5ac8282e1ea Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 15 Sep 2023 18:03:17 +0900 Subject: [PATCH 15/22] fix by review --- optuna_dashboard/ts/action.ts | 44 ++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 14 deletions(-) diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index 5b66c58f..1e75a3dc 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -611,23 +611,39 @@ export const actionCreator = () => { }) } - const removePreferentialHistory = (studyId: number, historyUuid: string) => { - removePreferentialHistoryAPI(studyId, historyUuid).catch((err) => { - const reason = err.response?.data.reason - enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { - variant: "error", + const removePreferentialHistory = (studyId: number, historyId: string) => { + removePreferentialHistoryAPI(studyId, historyId) + .then(() => { + const newStudy = Object.assign({}, studyDetails[studyId]) + newStudy.preference_history = newStudy.preference_history?.map((h) => + h.id === historyId ? { ...h, is_removed: true } : h + ) + setStudyDetailState(studyId, newStudy) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { + variant: "error", + }) + console.log(err) }) - console.log(err) - }) } - const restorePreferentialHistory = (studyId: number, historyUuid: string) => { - restorePreferentialHistoryAPI(studyId, historyUuid).catch((err) => { - const reason = err.response?.data.reason - enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { - variant: "error", + const restorePreferentialHistory = (studyId: number, historyId: string) => { + restorePreferentialHistoryAPI(studyId, historyId) + .then(() => { + const newStudy = Object.assign({}, studyDetails[studyId]) + newStudy.preference_history = newStudy.preference_history?.map((h) => + h.id === historyId ? { ...h, is_removed: false } : h + ) + setStudyDetailState(studyId, newStudy) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to switch history. Reason: ${reason}`, { + variant: "error", + }) + console.log(err) }) - console.log(err) - }) } return { From b7bd8ccd6e274ca779e4bf7b9ee3073bb5c5ec05 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 15 Sep 2023 18:17:38 +0900 Subject: [PATCH 16/22] fix bug --- .../ts/components/PreferentialTrials.tsx | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 5fc0ac32..feb845fc 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -187,12 +187,16 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ studyDetail, }) => { const [undoHistoryId, setUndoHistoryId] = useState(null) + const [displayTrials, setDisplayTrials] = useState({ + display: [], + clicked: [], + }) + const theme = useTheme() + const action = actionCreator() if (studyDetail === null || !studyDetail.is_preferential) { return null } - const theme = useTheme() - const action = actionCreator() const hiddenTrials = new Set( studyDetail.preference_history @@ -205,11 +209,6 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ (t.state === "Running" || t.state === "Complete") && !hiddenTrials.has(t.number) ) - - const [displayTrials, setDisplayTrials] = useState({ - display: [], - clicked: [], - }) const newTrials = activeTrials.filter( (t) => !displayTrials.display.includes(t.number) && From 9a73ef3631d08d2e4ee1597367bf98aff249d546 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 15 Sep 2023 18:24:32 +0900 Subject: [PATCH 17/22] fix by review --- optuna_dashboard/_app.py | 4 ++-- optuna_dashboard/_serializer.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/optuna_dashboard/_app.py b/optuna_dashboard/_app.py index 87efaae5..e13334d3 100644 --- a/optuna_dashboard/_app.py +++ b/optuna_dashboard/_app.py @@ -222,7 +222,7 @@ def create_app( plotly_graph_objects = get_plotly_graph_objects(system_attrs) skipped_trial_ids = get_skipped_trial_ids(system_attrs) - skipped_trials = [t.number for t in trials if t._trial_id in skipped_trial_ids] + skipped_trial_numbers = [t.number for t in trials if t._trial_id in skipped_trial_ids] return serialize_study_detail( summary, best_trials, @@ -232,7 +232,7 @@ def create_app( union_user_attrs, has_intermediate_values, plotly_graph_objects, - skipped_trials, + skipped_trial_numbers, ) @app.get("/api/studies//param_importances") diff --git a/optuna_dashboard/_serializer.py b/optuna_dashboard/_serializer.py index af3ed242..1f2b84bd 100644 --- a/optuna_dashboard/_serializer.py +++ b/optuna_dashboard/_serializer.py @@ -135,7 +135,7 @@ def serialize_study_detail( union_user_attrs: list[tuple[str, bool]], has_intermediate_values: bool, plotly_graph_objects: dict[str, str], - skipped_trials: list[int], + skipped_trial_numbers: list[int], ) -> dict[str, Any]: serialized: dict[str, Any] = { "name": summary.study_name, @@ -167,7 +167,7 @@ def serialize_study_detail( if serialized["is_preferential"]: serialized["preference_history"] = serialize_preference_history(system_attrs) serialized["preferences"] = get_preferences(system_attrs) - serialized["skipped_trial_numbers"] = skipped_trials + serialized["skipped_trial_numbers"] = skipped_trial_numbers serialized["plotly_graph_objects"] = [ {"id": id_, "graph_object": graph_object} for id_, graph_object in plotly_graph_objects.items() From 41081175f374f946667fd87fe4e93d1601f004d1 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Wed, 20 Sep 2023 18:34:16 +0900 Subject: [PATCH 18/22] fix undo in feedback screen --- .../ts/components/PreferentialTrials.tsx | 28 +++++++++++++++---- 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 64429451..1e05eaf0 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -401,7 +401,7 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ }) => { const theme = useTheme() const action = actionCreator() - const [undoHistoryId, setUndoHistoryId] = useState(null) + const [undoHistoryFlag, setUndoHistoryFlag] = useState(false) const [openThreejsArtifactModal, renderThreejsArtifactModal] = useThreejsArtifactModal() const [displayTrials, setDisplayTrials] = useState({ @@ -472,12 +472,23 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ } }) } + const visibleTrial = (num: number) => { + setDisplayTrials((prev) => { + const index = prev.clicked.findIndex((n) => n === num) + if (index === -1) { + return prev + } + const clicked = [...prev.clicked] + clicked[index] = -1 + return { + display: prev.display, + clicked: clicked, + } + }) + } const latestHistoryId = studyDetail?.preference_history?.filter((h) => !h.is_removed).pop()?.id ?? null - if (undoHistoryId !== null && undoHistoryId !== latestHistoryId) { - setUndoHistoryId(null) - } return ( @@ -500,13 +511,18 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ > From 1dd384933d0b131db68ef5a629874ad84e798f1b Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Thu, 21 Sep 2023 11:41:59 +0900 Subject: [PATCH 20/22] minor fix --- optuna_dashboard/ts/components/PreferentialTrials.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 85140011..b3934a80 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -510,6 +510,10 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ From 47539c37416bd3eaa45f6b7ba56b80adb0a42790 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 22 Sep 2023 15:32:44 +0900 Subject: [PATCH 21/22] fix by review --- optuna_dashboard/ts/action.ts | 28 ++++++++++++++----- optuna_dashboard/ts/apiClient.ts | 6 ++-- .../ts/components/PreferenceHistory.tsx | 10 +++---- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index a082d5fd..eb12abb1 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -589,17 +589,21 @@ export const actionCreator = () => { } const updatePreference = ( - study_id: number, + studyId: number, candidates: number[], clicked: number ) => { - reportPreferenceAPI(study_id, candidates, clicked).catch((err) => { - const reason = err.response?.data.reason - enqueueSnackbar(`Failed to report preference. Reason: ${reason}`, { - variant: "error", + reportPreferenceAPI(studyId, candidates, clicked) + .then(() => { + updateStudyDetail(studyId) + }) + .catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to report preference. Reason: ${reason}`, { + variant: "error", + }) + console.log(err) }) - console.log(err) - }) } const skipPreferentialTrial = (studyId: number, trialId: number) => { @@ -640,6 +644,12 @@ export const actionCreator = () => { newStudy.preference_history = newStudy.preference_history?.map((h) => h.id === historyId ? { ...h, is_removed: true } : h ) + const removed = newStudy.preference_history + ?.filter((h) => h.id === historyId) + .pop()?.preferences + newStudy.preferences = newStudy.preferences?.filter( + (p) => !removed?.some((r) => r[0] === p[0] && r[1] === p[1]) + ) setStudyDetailState(studyId, newStudy) }) .catch((err) => { @@ -658,6 +668,10 @@ export const actionCreator = () => { newStudy.preference_history = newStudy.preference_history?.map((h) => h.id === historyId ? { ...h, is_removed: false } : h ) + const restored = newStudy.preference_history + ?.filter((h) => h.id === historyId) + .pop()?.preferences + newStudy.preferences = newStudy.preferences?.concat(restored ?? []) setStudyDetailState(studyId, newStudy) }) .catch((err) => { diff --git a/optuna_dashboard/ts/apiClient.ts b/optuna_dashboard/ts/apiClient.ts index d6b3c77a..c0065bd3 100644 --- a/optuna_dashboard/ts/apiClient.ts +++ b/optuna_dashboard/ts/apiClient.ts @@ -55,7 +55,7 @@ const convertTrialResponse = (res: TrialResponse): Trial => { } } -interface PreferenceHistoryResponce { +interface PreferenceHistoryResponse { history: { id: string candidates: number[] @@ -68,7 +68,7 @@ interface PreferenceHistoryResponce { } const convertPreferenceHistory = ( - res: PreferenceHistoryResponce + res: PreferenceHistoryResponse ): PreferenceHistory => { return { id: res.history.id, @@ -97,7 +97,7 @@ interface StudyDetailResponse { objective_names?: string[] form_widgets?: FormWidgets preferences?: [number, number][] - preference_history?: PreferenceHistoryResponce[] + preference_history?: PreferenceHistoryResponse[] plotly_graph_objects: PlotlyGraphObject[] feedback_component_type: FeedbackComponentType skipped_trial_numbers?: number[] diff --git a/optuna_dashboard/ts/components/PreferenceHistory.tsx b/optuna_dashboard/ts/components/PreferenceHistory.tsx index a29f84b8..6aa67317 100644 --- a/optuna_dashboard/ts/components/PreferenceHistory.tsx +++ b/optuna_dashboard/ts/components/PreferenceHistory.tsx @@ -163,8 +163,8 @@ const CandidateTrial: FC<{ const ChoiceTrials: FC<{ choice: PreferenceHistory trials: Trial[] - study_id: number -}> = ({ choice, trials, study_id }) => { + studyId: number +}> = ({ choice, trials, studyId }) => { const [isRemoved, setRemoved] = useState(choice.is_removed) const theme = useTheme() const worst_trials = new Set([choice.clicked]) @@ -198,7 +198,7 @@ const ChoiceTrials: FC<{ disabled={!isRemoved} onClick={() => { setRemoved(false) - action.restorePreferentialHistory(study_id, choice.id) + action.restorePreferentialHistory(studyId, choice.id) }} sx={{ margin: `auto ${theme.spacing(2)}`, @@ -211,7 +211,7 @@ const ChoiceTrials: FC<{ disabled={isRemoved} onClick={() => { setRemoved(true) - action.removePreferentialHistory(study_id, choice.id) + action.removePreferentialHistory(studyId, choice.id) }} sx={{ margin: `auto ${theme.spacing(2)}`, @@ -279,7 +279,7 @@ export const PreferenceHistory: FC<{ studyDetail: StudyDetail | null }> = ({ key={choice.id} choice={choice} trials={studyDetail.trials} - study_id={studyDetail.id} + studyId={studyDetail.id} /> ))} From 02bfb7984d4a891b4a078775c62bb0586df21cc5 Mon Sep 17 00:00:00 2001 From: moririn2528 Date: Fri, 22 Sep 2023 17:16:02 +0900 Subject: [PATCH 22/22] fix by review --- optuna_dashboard/ts/action.ts | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/optuna_dashboard/ts/action.ts b/optuna_dashboard/ts/action.ts index eb12abb1..41afdadb 100644 --- a/optuna_dashboard/ts/action.ts +++ b/optuna_dashboard/ts/action.ts @@ -593,17 +593,13 @@ export const actionCreator = () => { candidates: number[], clicked: number ) => { - reportPreferenceAPI(studyId, candidates, clicked) - .then(() => { - updateStudyDetail(studyId) - }) - .catch((err) => { - const reason = err.response?.data.reason - enqueueSnackbar(`Failed to report preference. Reason: ${reason}`, { - variant: "error", - }) - console.log(err) + reportPreferenceAPI(studyId, candidates, clicked).catch((err) => { + const reason = err.response?.data.reason + enqueueSnackbar(`Failed to report preference. Reason: ${reason}`, { + variant: "error", }) + console.log(err) + }) } const skipPreferentialTrial = (studyId: number, trialId: number) => {