Skip to content

Commit

Permalink
Apply linter and replace trial_id in updatable check with trial_number
Browse files Browse the repository at this point in the history
  • Loading branch information
nabenabe0928 committed Dec 7, 2023
1 parent 3d45de9 commit c848401
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 12 deletions.
1 change: 0 additions & 1 deletion optuna_dashboard/_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,6 @@ def delete_study(study_id: int) -> dict[str, Any]:
@json_api_view
def get_study_detail(study_id: int) -> dict[str, Any]:
query_params = dict(after=0, limit=1000)
print(dict(request.params))
for query_key in query_params:
try:
query_params[query_key] = int(request.params[query_key])
Expand Down
19 changes: 9 additions & 10 deletions optuna_dashboard/_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from optuna.study import StudyDirection
from optuna.study import StudySummary
from optuna.trial import FrozenTrial
from optuna.trial import TrialState
from optuna.version import __version__ as optuna_ver
from packaging import version

Expand All @@ -27,19 +26,19 @@ def _should_update_trials_cache(storage: BaseStorage, study_id: int) -> bool:
if trials is None or len(trials) == 0:
return True

# TODO(nabenabe0928): Check any edge cases.
# 1. If some trials are still running or waiting?
# 2. If some trial_id after max_trial_id is missing?
updatable_states = [TrialState.RUNNING, TrialState.WAITING]
if any(t.state in updatable_states for t in trials):
first_updatable_id = min(t._trial_id for t in trials if t.state in updatable_states)
if any(not t.state.is_finished() for t in trials):
first_updatable_id = min(t._trial_id for t in trials if not t.state.is_finished())
first_updatable_trial = storage.get_trial(trial_id=first_updatable_id)
return first_updatable_trial.state not in updatable_states
return first_updatable_trial.state.is_finished()

Check warning on line 32 in optuna_dashboard/_storage.py

View check run for this annotation

Codecov / codecov/patch

optuna_dashboard/_storage.py#L29-L32

Added lines #L29 - L32 were not covered by tests

max_trial_id = max(t._trial_id for t in trials)
max_trial_number = max(t.number for t in trials)
try:

Check warning on line 35 in optuna_dashboard/_storage.py

View check run for this annotation

Codecov / codecov/patch

optuna_dashboard/_storage.py#L34-L35

Added lines #L34 - L35 were not covered by tests
# If another trial that did not exist is found in the database, nothing will be raised.
storage.get_trial(trial_id=max_trial_id + 1)
# For RDB, the time complexity of this query is O(log N)
storage.get_trial_id_from_study_id_trial_number(

Check warning on line 38 in optuna_dashboard/_storage.py

View check run for this annotation

Codecov / codecov/patch

optuna_dashboard/_storage.py#L38

Added line #L38 was not covered by tests
study_id=study_id,
trial_number=max_trial_number + 1,
)
return True
except KeyError:
return False

Check warning on line 44 in optuna_dashboard/_storage.py

View check run for this annotation

Codecov / codecov/patch

optuna_dashboard/_storage.py#L42-L44

Added lines #L42 - L44 were not covered by tests
Expand Down
2 changes: 1 addition & 1 deletion optuna_dashboard/ts/action.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ export const actionCreator = () => {
studyId in studyDetails
? studyDetails[studyId].trials.slice(0, firstUpdatableIndex)
: []
if (study.trials.length != 0){
if (study.trials.length !== 0){
study.trials = currentFixedTrials.concat(study.trials)
setStudyDetailState(studyId, study)
setIsTrialLeftInCache(study.fetched_trials_partially)
Expand Down

0 comments on commit c848401

Please sign in to comment.