Skip to content

Avoid deepcopy in _tell_with_warning#6079

Merged
gen740 merged 3 commits intooptuna:masterfrom
sawa3030:avoid-deepcopy
May 30, 2025
Merged

Avoid deepcopy in _tell_with_warning#6079
gen740 merged 3 commits intooptuna:masterfrom
sawa3030:avoid-deepcopy

Conversation

@sawa3030
Copy link
Copy Markdown
Collaborator

@sawa3030 sawa3030 commented May 9, 2025

Motivation

Improve the performance of _tell_with_warning by reducing unnecessary deep copies.

Description of the Changes

  • Replaced deepcopy with a shallow copy in _tell_with_warning to reduce overhead when returning a FrozenTrial.
  • A deepcopy is performed only in contexts where the FrozenTrial might be modified, such as when it is passed into callbacks or used in study.tell().

@sawa3030
Copy link
Copy Markdown
Collaborator Author

sawa3030 commented May 9, 2025

Performance Improvement

The test was conducted by alternating between the master branch and this PR for 10 iterations. The results show a slight improvement in the cumulative time (cumtime) of _run_trial, as expected.
note: Because the performance difference is subtle, results from a single iteration was heavily influenced by external factors and are therefore not reliable on their own.

Branch Average Cumtime
master 16.6892 ± 0.43999
this PR 16.4755 ± 0.48987

The code used for testing is below.

import optuna
import cProfile

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()
profiller = cProfile.Profile()
profiller.run('study.optimize(objective, n_trials=500)')

@sawa3030 sawa3030 marked this pull request as ready for review May 9, 2025 07:06
@not522
Copy link
Copy Markdown
Member

not522 commented May 12, 2025

There is a significant speed difference with RandomSampler.

  • master 1.50s user 0.06s system 93% cpu 1.681 total
  • PR 1.34s user 0.07s system 87% cpu 1.622 total
import optuna

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())
study.optimize(objective, n_trials=10000)

@c-bata c-bata added the enhancement Change that does not break compatibility and not affect public interfaces, but improves performance. label May 13, 2025
@c-bata
Copy link
Copy Markdown
Member

c-bata commented May 13, 2025

@gen740 @not522 Could you review this PR?

frozen_trial = study._storage.get_trial(frozen_trial._trial_id)

if warning_message is not None:
frozen_trial._system_attrs[STUDY_TELL_WARNING_KEY] = warning_message
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

frozen_trial is modified in this line. So, could you refactor the warning message implementation first?

@sawa3030
Copy link
Copy Markdown
Collaborator Author

@not522 Thank you for the suggestion. I have opened a new PR #6082, so I will close this PR for now. Once #6082 is merged, I will reopen this PR accordingly.

@sawa3030 sawa3030 closed this May 14, 2025
@sawa3030 sawa3030 reopened this May 23, 2025
@sawa3030
Copy link
Copy Markdown
Collaborator Author

Let me reopen this PR since #6082 is merged into master

@not522
Copy link
Copy Markdown
Member

not522 commented May 27, 2025

Could you also add deepcopy to Study.tell?

Comment on lines 171 to 172
frozen_trial = copy.deepcopy(frozen_trial)
callback(study, frozen_trial)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current implementation, if multiple callbacks are passed, modifying frozen_trial within any callback will affect the frozen_trial passed to subsequent callbacks.

Suggested change
frozen_trial = copy.deepcopy(frozen_trial)
callback(study, frozen_trial)
callback(study, copy.deepcopy(frozen_trial))

@sawa3030
Copy link
Copy Markdown
Collaborator Author

Thank you for your feedback. I've incorporated the suggested changes. PTAL

Copy link
Copy Markdown
Member

@not522 not522 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@not522 not522 removed their assignment May 29, 2025
Copy link
Copy Markdown
Member

@gen740 gen740 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@gen740 gen740 added this to the v4.4.0 milestone May 30, 2025
@gen740 gen740 merged commit 29f19f1 into optuna:master May 30, 2025
14 checks passed
@gen740 gen740 removed their assignment May 30, 2025
@sawa3030 sawa3030 deleted the avoid-deepcopy branch November 14, 2025 07:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Change that does not break compatibility and not affect public interfaces, but improves performance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants