diff --git a/examples/preferential-optimization/generator.py b/examples/preferential-optimization/generator.py index e94f1d05..d8f7d3bd 100644 --- a/examples/preferential-optimization/generator.py +++ b/examples/preferential-optimization/generator.py @@ -64,9 +64,6 @@ def main() -> NoReturn: ) save_note(trial, note) - # 5. Mark comparison ready - study.mark_comparison_ready(trial) - if __name__ == "__main__": main() diff --git a/optuna_dashboard/preferential/_study.py b/optuna_dashboard/preferential/_study.py index caa1896d..6e093683 100644 --- a/optuna_dashboard/preferential/_study.py +++ b/optuna_dashboard/preferential/_study.py @@ -14,6 +14,7 @@ from optuna.trial import FrozenTrial from optuna.trial import TrialState from optuna_dashboard.preferential._system_attrs import get_n_generate from optuna_dashboard.preferential._system_attrs import get_preferences +from optuna_dashboard.preferential._system_attrs import get_skipped_trial_ids from optuna_dashboard.preferential._system_attrs import is_skipped_trial from optuna_dashboard.preferential._system_attrs import report_preferences from optuna_dashboard.preferential._system_attrs import set_n_generate @@ -21,7 +22,6 @@ from optuna_dashboard.preferential._system_attrs import set_n_generate _logger = logging.get_logger(__name__) _SYSTEM_ATTR_PREFERENTIAL_STUDY = "preference:is_preferential" -_SYSTEM_ATTR_COMPARISON_READY = "preference:comparison_ready" class PreferentialStudy: @@ -62,13 +62,6 @@ class PreferentialStudy: def best_trials(self) -> list[FrozenTrial]: """Return the trials that is not dominated by other trials. - .. seealso:: - - See `Study.best_trials`_ for details. - - .. _Study.best_trials: https://optuna.readthedocs.io/en/stable/reference/\ - generated/optuna.study.Study.html#optuna.study.Study.best_trials - Returns: A list of FrozenTrial object """ @@ -251,8 +244,11 @@ class PreferentialStudy: Returns: A list of the pair of FrozenTrial objects. The left trial is better than the right one. """ + + preferences = get_preferences( + self._study._storage.get_study_system_attrs(self._study._study_id) + ) # Must come before study.get_trials() trials = self._study.get_trials(deepcopy=deepcopy) - preferences = get_preferences(self._study._study_id, self._study._storage) return [(trials[better], trials[worse]) for (better, worse) in preferences] def set_user_attr(self, key: str, value: Any) -> None: @@ -269,24 +265,6 @@ class PreferentialStudy: """ self._study.set_user_attr(key, value) - def mark_comparison_ready(self, trial_or_number: optuna.Trial | int) -> None: - """Mark trials ready to compare. - - Args: - trial_or_number: - A Trial object or trial_number. - """ - storage = self._study._storage - if isinstance(trial_or_number, optuna.Trial): - trial_id = trial_or_number._trial_id - elif isinstance(trial_or_number, int): - trial_id = storage.get_trial_id_from_study_id_trial_number( - self._study._study_id, trial_or_number - ) - else: - raise RuntimeError("Unexpected trial type") - storage.set_trial_system_attr(trial_id, _SYSTEM_ATTR_COMPARISON_READY, True) - def should_generate(self) -> bool: """Return whether the generator should generate a new trial now. @@ -295,21 +273,33 @@ class PreferentialStudy: to generate a new trial if this method returns :obj:`True`, and to wait for human evaluation if this method returns :obj:`False`. """ - return len(self.best_trials) < get_n_generate(self._study.system_attrs) + study_system_attrs = self._study._storage.get_study_system_attrs( + self._study._study_id + ) # Must come before _study.get_trials() + trials = self._study.get_trials( + deepcopy=False, states=(TrialState.COMPLETE, TrialState.RUNNING) + ) + worse_trial_numbers = {worse for _, worse in get_preferences(study_system_attrs)} + skipped_trial_ids = set(get_skipped_trial_ids(study_system_attrs)) + active_trials = [ + t + for t in trials + if t.number not in worse_trial_numbers and t._trial_id not in skipped_trial_ids + ] + return len(active_trials) < get_n_generate(self._study.system_attrs) def get_best_trials(study_id: int, storage: optuna.storages.BaseStorage) -> list[FrozenTrial]: - preferences = get_preferences(study_id, storage) + preferences = get_preferences(storage.get_study_system_attrs(study_id)) worse_numbers = {worse for _, worse in preferences} + nondominated_numbers = {better for better, _ in preferences if better not in worse_numbers} + trials = storage.get_all_trials(study_id, deepcopy=False) + study_system_attrs = storage.get_study_system_attrs(study_id) + best_trials = [] - for t in storage.get_all_trials( - study_id, deepcopy=False, states=(TrialState.COMPLETE, TrialState.RUNNING) - ): - if not t.system_attrs.get(_SYSTEM_ATTR_COMPARISON_READY, False): - continue - if t.number in worse_numbers: - continue + for n in nondominated_numbers: + t = trials[n] if is_skipped_trial(t._trial_id, study_system_attrs): continue best_trials.append(copy.deepcopy(t)) diff --git a/optuna_dashboard/preferential/_system_attrs.py b/optuna_dashboard/preferential/_system_attrs.py index 4cb0e288..47c2a486 100644 --- a/optuna_dashboard/preferential/_system_attrs.py +++ b/optuna_dashboard/preferential/_system_attrs.py @@ -35,13 +35,9 @@ def report_preferences( return preference_id -def get_preferences( - study_id: int, - storage: BaseStorage, -) -> list[tuple[int, int]]: +def get_preferences(study_system_attrs: dict[str, Any]) -> list[tuple[int, int]]: preferences: list[tuple[int, int]] = [] - system_attrs = storage.get_study_system_attrs(study_id) - for k, v in system_attrs.items(): + for k, v in study_system_attrs.items(): if not k.startswith(_SYSTEM_ATTR_PREFIX_PREFERENCE): continue preferences.extend(v) # type: ignore @@ -65,6 +61,19 @@ def is_skipped_trial(trial_id: int, study_system_attrs: dict[str, Any]) -> bool: return key in study_system_attrs +def get_skipped_trial_ids(study_system_attrs: dict[str, Any]) -> list[int]: + skipped_trial_ids: list[int] = [] + for k in study_system_attrs: + if not k.startswith(_SYSTEM_ATTR_PREFIX_SKIP_TRIAL): + continue + try: + trial_id = int(k[len(_SYSTEM_ATTR_PREFIX_SKIP_TRIAL) :]) # noqa: E203 + skipped_trial_ids.append(trial_id) + except ValueError: + continue + return skipped_trial_ids + + def get_n_generate(study_system_attrs: dict[str, Any]) -> int: return study_system_attrs[_SYSTEM_ATTR_N_GENERATE] diff --git a/optuna_dashboard/preferential/samplers/gp.py b/optuna_dashboard/preferential/samplers/gp.py index ff4001c1..b90a1de4 100644 --- a/optuna_dashboard/preferential/samplers/gp.py +++ b/optuna_dashboard/preferential/samplers/gp.py @@ -342,7 +342,7 @@ class PreferentialGPSampler(optuna.samplers.BaseSampler): if len(search_space) == 0: return {} - preferences = get_preferences(study._study_id, study._storage) + preferences = get_preferences(study.system_attrs) trials = study.get_trials(deepcopy=False) if len(preferences) == 0: return {} diff --git a/optuna_dashboard/ts/components/PreferentialTrials.tsx b/optuna_dashboard/ts/components/PreferentialTrials.tsx index 93ed2f5a..08efd9a7 100644 --- a/optuna_dashboard/ts/components/PreferentialTrials.tsx +++ b/optuna_dashboard/ts/components/PreferentialTrials.tsx @@ -42,6 +42,8 @@ const PreferentialTrial: FC<{ ) } + const isBestTrial = trial.state === "Complete" + return ( true} + isBestTrial={() => isBestTrial} directions={[]} objectiveNames={[]} /> @@ -182,11 +184,15 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ return null } const theme = useTheme() + + const runningTrials = studyDetail.trials.filter((t) => t.state === "Running") + const activeTrials = runningTrials.concat(studyDetail.best_trials) + const [displayTrials, setDisplayTrials] = useState({ - numbers: studyDetail.best_trials.map((t) => t.number), - last_number: Math.max(...studyDetail.best_trials.map((t) => t.number), -1), + numbers: activeTrials.map((t) => t.number), + last_number: Math.max(...activeTrials.map((t) => t.number), -1), }) - const new_trails = studyDetail.best_trials.filter( + const new_trails = activeTrials.filter( (t) => displayTrials.last_number < t.number && displayTrials.numbers.find((n) => n === t.number) === undefined @@ -239,7 +245,7 @@ export const PreferentialTrials: FC<{ studyDetail: StudyDetail | null }> = ({ {displayTrials.numbers.map((t, index) => ( trial.number === t)} + trial={activeTrials.find((trial) => trial.number === t)} candidates={displayTrials.numbers.filter((n) => n !== -1)} hideTrial={() => { hideTrial(t) diff --git a/python_tests/preferential/test_study.py b/python_tests/preferential/test_study.py index 3de57dae..7fc1aaba 100644 --- a/python_tests/preferential/test_study.py +++ b/python_tests/preferential/test_study.py @@ -40,7 +40,6 @@ def test_report_and_get_preferences(storage_supplier: Callable[[], StorageSuppli for _ in range(2): trial = study.ask() trial.suggest_float("x", 0, 1) - study.mark_comparison_ready(trial) better, worse = study.trials study.report_preference(better, worse) assert len(study.preferences) == 1 @@ -152,7 +151,6 @@ def test_copy_study() -> None: for _ in range(3): trial = from_study.ask() trial.suggest_float("x", 0, 1) - from_study.mark_comparison_ready(trial) from_study.report_preference(from_study.trials[0], from_study.trials[1]) from_study.report_preference(from_study.trials[1], from_study.trials[2]) @@ -243,7 +241,6 @@ def test_get_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: for _ in range(5): trial = study.ask() trial.suggest_int("x", 1, 5) - study.mark_comparison_ready(trial) with patch("copy.deepcopy", wraps=copy.deepcopy) as mock_object: trials0 = study.get_trials(deepcopy=False) @@ -266,8 +263,7 @@ def test_get_trials_state_option(storage_supplier: Callable[[], StorageSupplier] with storage_supplier() as storage: study = create_study(n_generate=4, storage=storage) for _ in range(3): - trial = study.ask() - study.mark_comparison_ready(trial) + study.ask() better, worse = study.trials[:2] study.report_preference(better, worse) diff --git a/python_tests/preferential/test_system_attrs.py b/python_tests/preferential/test_system_attrs.py index 10448d48..34f93200 100644 --- a/python_tests/preferential/test_system_attrs.py +++ b/python_tests/preferential/test_system_attrs.py @@ -18,12 +18,13 @@ def test_report_and_get_preferences(storage_supplier: Callable[[], StorageSuppli study.ask() study_id = study._study_id - assert len(get_preferences(study_id, storage)) == 0 + + assert len(get_preferences(storage.get_study_system_attrs(study_id))) == 0 better, worse = study.trials[0], study.trials[1] report_preferences(study_id, storage, [(better.number, worse.number)]) - assert len(get_preferences(study_id, storage)) == 1 + assert len(get_preferences(storage.get_study_system_attrs(study_id))) == 1 - actual_better, actual_worse = get_preferences(study_id, storage)[0] + actual_better, actual_worse = get_preferences(storage.get_study_system_attrs(study_id))[0] assert actual_better == better.number assert actual_worse == worse.number diff --git a/python_tests/test_api.py b/python_tests/test_api.py index e50f2501..c551e3f2 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -104,10 +104,11 @@ class APITestCase(TestCase): storage = optuna.storages.InMemoryStorage() study = create_study(n_generate=4, storage=storage) for _ in range(3): - trial = study.ask() - study.mark_comparison_ready(trial) + study.ask() study.report_preference(study.trials[0], study.trials[1]) + assert len(study.best_trials) == 1 + app = create_app(storage) study_id = study._study._study_id status, _, body = send_request( @@ -119,16 +120,14 @@ class APITestCase(TestCase): self.assertEqual(status, 200) best_trials = json.loads(body)["best_trials"] - assert len(best_trials) == 2 + assert len(best_trials) == 1 assert best_trials[0]["number"] == 0 - assert best_trials[1]["number"] == 2 def test_report_preference(self) -> None: storage = optuna.storages.InMemoryStorage() study = create_study(n_generate=4, storage=storage) for _ in range(3): - trial = study.ask() - study.mark_comparison_ready(trial) + study.ask() app = create_app(storage) study_id = study._study._study_id @@ -161,8 +160,7 @@ class APITestCase(TestCase): storage = optuna.storages.InMemoryStorage() 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 @@ -187,23 +185,23 @@ class APITestCase(TestCase): trials: list[optuna.Trial] = [] for _ in range(3): trial = study.ask() - study.mark_comparison_ready(trial) trials.append(trial) + study.report_preference(study.trials[0], study.trials[1]) + study.report_preference(study.trials[2], study.trials[1]) app = create_app(storage) study_id = study._study._study_id status, _, _ = send_request( app, - f"/api/studies/{study_id}/{trials[1]._trial_id}/skip", + f"/api/studies/{study_id}/{trials[0]._trial_id}/skip", "POST", content_type="application/json", ) self.assertEqual(status, 204) best_trials = study.best_trials - assert len(best_trials) == 2 - assert best_trials[0].number == 0 - assert best_trials[1].number == 2 + assert len(best_trials) == 1 + assert best_trials[0].number == 2 def test_create_study(self) -> None: for name, directions, expected_status in [ diff --git a/python_tests/test_preferential_history.py b/python_tests/test_preferential_history.py index ab524b90..51c9b0f8 100644 --- a/python_tests/test_preferential_history.py +++ b/python_tests/test_preferential_history.py @@ -19,7 +19,6 @@ def test_report_and_get_choices(storage_supplier: Callable[[], StorageSupplier]) for _ in range(5): trial = study.ask() trial.suggest_float("x", 0, 1) - study.mark_comparison_ready(trial) study_id = study._study._study_id