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