Skip to content

Refactor tell_with_warning to avoid unnecessary get_trial call #6133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sawa3030
Copy link
Collaborator

@sawa3030 sawa3030 commented Jun 6, 2025

Motivation

Remove unnecessary calls to storage.get_trial to improve efficiency.

Description of the changes

  • Removed the final storage.get_trial call in tell_with_warning.
  • Instead of returning the FrozenTrial, tell_with_warning now returns state, values, params, and values_conversion_failure_message, which are required for subsequent log output.

@sawa3030
Copy link
Collaborator Author

sawa3030 commented Jun 6, 2025

Performance Improvement

The runtime of _run_trial was compared between this PR and the main branch using the following code. The results shows a noticeable improvement in the cumulative time (cumtime) of _run_trial, as expected.

Branch Average Cumtime
master 37.3786 ± 0.5339
this PR 40.8345 ± 0.8265
import optuna
import optunahub
import cProfile

storage = optuna.storages.RDBStorage(
    url="sqlite:///:memory:",
    engine_kwargs={"pool_size": 20, "connect_args": {"timeout": 10}},
)

def objective(trial: optuna.Trial) -> float:
    x = trial.suggest_float("x", -1, 1)
    y = trial.suggest_int("y", -1, 1)
    return x**2 + y

study = optuna.create_study(sampler=optuna.samplers.RandomSampler(), storage=storage)

profiller = cProfile.Profile()
study.optimize(objective, n_trials=500)

@sawa3030 sawa3030 marked this pull request as ready for review June 6, 2025 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants