-
-
Notifications
You must be signed in to change notification settings - Fork 960
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
Fixing potential unbound local variable in Study._optimize_sequential
#5280
Comments
Could you share the minimal reproducible code to see the error with us? |
I don't have anything minimal right now but I'll try to come up with something |
It seems I was hasty with the above code change suggestion: when Here is the relevant snippet from the logs of one of my failing runs
The error originates within from typing import Optional, Sequence
import optuna
from json import JSONDecodeError
from optuna.storages._journal.base import BaseJournalLogStorage
from optuna.trial import TrialState
def trial_fn(trial: optuna.Trial):
foo = trial.suggest_float("foo", 0., 1.)
return foo
class ErroneousJournalStorage(optuna.storages.JournalStorage):
def __init__(self, log_storage: BaseJournalLogStorage) -> None:
self.trial_state_values_set = False
super().__init__(log_storage)
def set_trial_state_values(self, trial_id: int, state: TrialState, values: Optional[Sequence[float]] = None) -> bool:
self.trial_state_values_set = True
return super().set_trial_state_values(trial_id, state, values)
def _sync_with_backend(self) -> None:
if self.trial_state_values_set:
raise JSONDecodeError("some json error", "foo", 0)
return super()._sync_with_backend()
sampler = optuna.samplers.TPESampler()
storage = ErroneousJournalStorage(
optuna.storages.JournalFileStorage("foo"),
)
study = optuna.create_study(storage=storage, sampler=sampler)
study.optimize(trial_fn, n_trials=100, gc_after_trial=True) Related problemAnother issue I was seeing appears to be when try:
frozen_trial = _tell_with_warning(
study=study,
trial=trial,
value_or_values=value_or_values,
state=state,
suppress_warning=True,
)
except Exception:
frozen_trial = study._storage.get_trial(trial._trial_id)
raise in This can be reproduced by using the above code with a modified class ErroneousJournalStorage(optuna.storages.JournalStorage):
def set_trial_state_values(self, trial_id: int, state: TrialState, values: Optional[Sequence[float]] = None) -> bool:
raise JSONDecodeError("some json error", "foo", 0) |
So can we close this issue? |
Well, no. There is an issue with unhandled errors here after all.. You can rename/relabel as you wish, but the problem will not go away by closing this, aye? |
I am also experiencing this error constantly, however it's not so easy to reproduce since it happens after some hours of training. It started when I migrated from a local sqlite DB to a postgresql accessed through the RDBStorage object. My traceback is very similar but not identical:
|
Motivation
Study._optimize_sequential
encounters an issue if_run_trial
errors and callbacks are present, as seen in the following relevant code block:If
_run_trial
raises,frozen_trial
is never set, leading to anUnboundLocalError: local variable 'frozen_trial' referenced before assignment
error.Suggestion
Consider moving the callback loop into the try block? Not sure if there might be some side effects in moving this before the garbage collection, however.
Additional context (optional)
This really is a bug report, I saw this as a cascading error after the file based journal encountered a
json.decoder.JSONDecodeError
. However, the bug report template is asking for so many details irrelevant to this that filling this template is the more feasible alternative.The text was updated successfully, but these errors were encountered: