diff --git a/examples/preferential-optimization/generator.py b/examples/preferential-optimization/generator.py index 1a498fe7..d8f7d3bd 100644 --- a/examples/preferential-optimization/generator.py +++ b/examples/preferential-optimization/generator.py @@ -23,6 +23,7 @@ os.makedirs(artifact_path, exist_ok=True) def main() -> NoReturn: study = create_study( + n_generate=5, study_name="Preferential Optimization", storage=STORAGE_URL, sampler=PreferentialGPSampler(), @@ -63,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 8895d604..e412d0ee 100644 --- a/optuna_dashboard/preferential/_study.py +++ b/optuna_dashboard/preferential/_study.py @@ -21,7 +21,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: @@ -230,44 +229,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) - - @property - def n_generate(self) -> int: - """Return the number of trials that should be generated and shown to user. - - :func:`~optuna_dashboard.preferential.PreferentialStudy.should_generate` returns - :obj:`True` if the number of trials not reported bad and not skipped are less than - :attr:`~optuna_dashboard.preferential.PreferentialStudy.n_generate`. - """ - system_attrs = self._study._storage.get_study_system_attrs(self._study._study_id) - return get_n_generate(system_attrs) - - def set_n_generate(self, n_generate: int) -> None: - """Set the number of trials that should be generated and shown to user. - - :func:`~optuna_dashboard.preferential.PreferentialStudy.should_generate` returns - :obj:`True` if the number of trials not reported bad and not skipped are less than - :attr:`~optuna_dashboard.preferential.PreferentialStudy.n_generate`. - """ - return set_n_generate(self._study._study_id, self._study._storage, n_generate) - def should_generate(self) -> bool: """Return whether the generator should generate a new trial now. @@ -276,25 +237,7 @@ 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(get_active_trials(self._study._study_id, self._study._storage)) < self.n_generate - ) - - -def get_active_trials(study_id: int, storage: optuna.storages.BaseStorage) -> list[FrozenTrial]: - preferences = get_preferences(study_id, storage) - worse_numbers = {worse for _, worse in preferences} - study_system_attrs = storage.get_study_system_attrs(study_id) - active_trials = [] - for t in storage.get_all_trials( - study_id, deepcopy=False, states=(TrialState.COMPLETE, TrialState.RUNNING) - ): - if t.number in worse_numbers: - continue - if is_skipped_trial(t._trial_id, study_system_attrs): - continue - active_trials.append(copy.deepcopy(t)) - return active_trials + return len(self.best_trials) < get_n_generate(self._study.system_attrs) def get_best_trials(study_id: int, storage: optuna.storages.BaseStorage) -> list[FrozenTrial]: @@ -318,6 +261,7 @@ def get_best_trials(study_id: int, storage: optuna.storages.BaseStorage) -> list def create_study( *, + n_generate: int, storage: str | optuna.storages.BaseStorage | None = None, sampler: BaseSampler | None = None, study_name: str | None = None, @@ -337,6 +281,12 @@ def create_study( trial = study.ask() Args: + n_generate: + The number of active trials to keep. + :func:`~optuna_dashboard.preferential.PreferentialStudy.should_generate` returns + :obj:`True` if the number of trials not reported bad and not skipped are less than + ``n_generate``. + storage: Database URL. If this argument is set to None, in-memory storage is used, and the :class:`~optuna_dashboard.preferential.PreferentialStudy` will not be persistent. @@ -372,7 +322,7 @@ def create_study( study._storage.set_study_system_attr( study._study_id, _SYSTEM_ATTR_PREFERENTIAL_STUDY, True ) - set_n_generate(study._study_id, study._storage, 4) # Default n_generate + set_n_generate(study._study_id, study._storage, n_generate) return PreferentialStudy(study) except optuna.exceptions.DuplicatedStudyError: diff --git a/python_tests/preferential/test_study.py b/python_tests/preferential/test_study.py index 3e0b011c..abc1bc4d 100644 --- a/python_tests/preferential/test_study.py +++ b/python_tests/preferential/test_study.py @@ -25,7 +25,7 @@ from ..storage_supplier import StorageSupplier @parametrize_storages def test_study_set_and_get_user_attrs(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) study.set_user_attr("dataset", "MNIST") assert study.user_attrs["dataset"] == "MNIST" @@ -34,13 +34,12 @@ def test_study_set_and_get_user_attrs(storage_supplier: Callable[[], StorageSupp @parametrize_storages def test_report_and_get_preferences(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) assert len(study.preferences) == 0 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 @@ -51,7 +50,9 @@ def test_report_and_get_preferences(storage_supplier: Callable[[], StorageSuppli def test_study_pickle() -> None: - study_1 = create_study() + study_1 = create_study( + n_generate=4, + ) for _ in range(10): study_1.ask() assert len(study_1.trials) == 10 @@ -69,13 +70,17 @@ def test_study_pickle() -> None: def test_create_study(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: # Test creating a new study. - study = create_study(storage=storage, load_if_exists=False) + study = create_study(n_generate=4, storage=storage, load_if_exists=False) # Test `load_if_exists=True` with existing study. - create_study(study_name=study.study_name, storage=storage, load_if_exists=True) + create_study( + n_generate=4, study_name=study.study_name, storage=storage, load_if_exists=True + ) with pytest.raises(DuplicatedStudyError): - create_study(study_name=study.study_name, storage=storage, load_if_exists=False) + create_study( + n_generate=4, study_name=study.study_name, storage=storage, load_if_exists=False + ) @parametrize_storages @@ -92,7 +97,7 @@ def test_load_study(storage_supplier: Callable[[], StorageSupplier]) -> None: load_study(study_name=study_name, storage=storage) # Create a new study. - created_study = create_study(study_name=study_name, storage=storage) + created_study = create_study(n_generate=4, study_name=study_name, storage=storage) # Test loading an existing study. loaded_study = load_study(study_name=study_name, storage=storage) @@ -108,7 +113,7 @@ def test_load_study_study_name_none(storage_supplier: Callable[[], StorageSuppli study_name = str(uuid.uuid4()) - _ = create_study(study_name=study_name, storage=storage) + _ = create_study(n_generate=4, study_name=study_name, storage=storage) loaded_study = load_study(study_name=None, storage=storage) @@ -116,7 +121,7 @@ def test_load_study_study_name_none(storage_supplier: Callable[[], StorageSuppli study_name = str(uuid.uuid4()) - _ = create_study(study_name=study_name, storage=storage) + _ = create_study(n_generate=4, study_name=study_name, storage=storage) # Ambiguous study. with pytest.raises(ValueError): @@ -131,7 +136,7 @@ def test_delete_study(storage_supplier: Callable[[], StorageSupplier]) -> None: delete_study(study_name="invalid-study-name", storage=storage) # Test deleting an existing study. - study = create_study(storage=storage, load_if_exists=False) + study = create_study(n_generate=4, storage=storage, load_if_exists=False) delete_study(study_name=study.study_name, storage=storage) # Test failed to delete the study which is already deleted. @@ -141,12 +146,11 @@ def test_delete_study(storage_supplier: Callable[[], StorageSupplier]) -> None: def test_copy_study() -> None: with StorageSupplier("sqlite") as from_storage, StorageSupplier("sqlite") as to_storage: - from_study = create_study(storage=from_storage) + from_study = create_study(n_generate=4, storage=from_storage) from_study.set_user_attr("baz", "qux") 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]) @@ -165,8 +169,8 @@ def test_copy_study() -> None: def test_copy_study_to_study_name() -> None: with StorageSupplier("sqlite") as from_storage, StorageSupplier("sqlite") as to_storage: - from_study = create_study(study_name="foo", storage=from_storage) - _ = create_study(study_name="foo", storage=to_storage) + from_study = create_study(n_generate=4, study_name="foo", storage=from_storage) + _ = create_study(n_generate=4, study_name="foo", storage=to_storage) with pytest.raises(DuplicatedStudyError): copy_study( @@ -188,7 +192,7 @@ def test_copy_study_to_study_name() -> None: @parametrize_storages def test_add_trial(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) assert len(study.trials) == 0 trial = create_trial(value=0) @@ -198,7 +202,9 @@ def test_add_trial(storage_supplier: Callable[[], StorageSupplier]) -> None: def test_add_trial_invalid_values_length() -> None: - study = create_study() + study = create_study( + n_generate=4, + ) trial = create_trial(values=[0, 0]) with pytest.raises(ValueError): study.add_trial(trial) @@ -207,7 +213,7 @@ def test_add_trial_invalid_values_length() -> None: @parametrize_storages def test_add_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) assert len(study.trials) == 0 study.add_trials([]) @@ -220,7 +226,7 @@ def test_add_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: assert trial.number == i assert trial.value == i - other_study = create_study(storage=storage) + other_study = create_study(n_generate=4, storage=storage) other_study.add_trials(study.trials) assert len(other_study.trials) == 3 for i, trial in enumerate(other_study.trials): @@ -231,11 +237,10 @@ def test_add_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: @parametrize_storages def test_get_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) 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) @@ -256,10 +261,9 @@ def test_get_trials(storage_supplier: Callable[[], StorageSupplier]) -> None: @parametrize_storages def test_get_trials_state_option(storage_supplier: Callable[[], StorageSupplier]) -> None: with storage_supplier() as storage: - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) for _ in range(3): trial = study.ask() - study.mark_comparison_ready(trial) better, worse = study.trials[:2] study.report_preference(better, worse) @@ -286,7 +290,9 @@ def test_get_trials_state_option(storage_supplier: Callable[[], StorageSupplier] def test_ask() -> None: - study = create_study() + study = create_study( + n_generate=4, + ) trial = study.ask() assert isinstance(trial, Trial) @@ -298,7 +304,9 @@ def test_ask_fixed_search_space() -> None: "y": distributions.CategoricalDistribution(["bacon", "spam"]), } - study = create_study() + study = create_study( + n_generate=4, + ) trial = study.ask(fixed_distributions=fixed_distributions) params = trial.params @@ -312,7 +320,7 @@ def test_report_preferences_from_another_process() -> None: with StorageSupplier("sqlite") as storage: # Create a study and ask for a new trial. - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) study.ask() study.ask() diff --git a/python_tests/test_api.py b/python_tests/test_api.py index fc67e12e..ba3b0dc4 100644 --- a/python_tests/test_api.py +++ b/python_tests/test_api.py @@ -102,10 +102,9 @@ class APITestCase(TestCase): def test_get_best_trials_of_preferential_study(self) -> None: storage = optuna.storages.InMemoryStorage() - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) for _ in range(3): trial = study.ask() - study.mark_comparison_ready(trial) study.report_preference(study.trials[0], study.trials[1]) assert len(study.best_trials) == 1 @@ -126,10 +125,9 @@ class APITestCase(TestCase): def test_report_preference(self) -> None: storage = optuna.storages.InMemoryStorage() - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) for _ in range(3): trial = study.ask() - study.mark_comparison_ready(trial) app = create_app(storage) study_id = study._study._study_id @@ -154,11 +152,10 @@ class APITestCase(TestCase): def test_skip_trial(self) -> None: storage = optuna.storages.InMemoryStorage() - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) trials: list[optuna.Trial] = [] for _ in range(3): trial = study.ask() - study.mark_comparison_ready(trial) trials.append(trial) app = create_app(storage) diff --git a/python_tests/test_serializers.py b/python_tests/test_serializers.py index a75db32d..a90e0de7 100644 --- a/python_tests/test_serializers.py +++ b/python_tests/test_serializers.py @@ -24,7 +24,7 @@ def test_serialize_dict() -> None: def test_get_study_detail_is_preferential() -> None: storage = optuna.storages.InMemoryStorage() - study = create_study(storage=storage) + study = create_study(n_generate=4, storage=storage) study_summaries = get_study_summaries(storage) assert len(study_summaries) == 1 @@ -46,7 +46,7 @@ def test_get_study_detail_is_not_preferential() -> None: def test_get_study_summary_is_preferential() -> None: storage = optuna.storages.InMemoryStorage() - create_study(storage=storage) + create_study(n_generate=4, storage=storage) study_summaries = get_study_summaries(storage) assert len(study_summaries) == 1