Skip to content

Update warning message and add a test when a trial fails with exception#3454

Merged
not522 merged 2 commits intooptuna:masterfrom
himkt:study-tell-followup
Apr 25, 2022
Merged

Update warning message and add a test when a trial fails with exception#3454
not522 merged 2 commits intooptuna:masterfrom
himkt:study-tell-followup

Conversation

@himkt
Copy link
Copy Markdown
Member

@himkt himkt commented Apr 7, 2022

Followup of #3144.

Description of the changes

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #3454 (0cc79eb) into master (217dd5e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #3454   +/-   ##
=======================================
  Coverage   91.86%   91.86%           
=======================================
  Files         157      157           
  Lines       12317    12319    +2     
=======================================
+ Hits        11315    11317    +2     
  Misses       1002     1002           
Impacted Files Coverage Δ
optuna/study/_optimize.py 98.63% <100.00%> (+<0.01%) ⬆️
optuna/study/_tell.py 97.95% <100.00%> (+0.02%) ⬆️

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@HideakiImamura
Copy link
Copy Markdown
Member

@not522 Could you review this PR?

Copy link
Copy Markdown
Member

@HideakiImamura HideakiImamura left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Looks good to me. (The STUDY_TELL_WARNING_KEY = "STUDY_TELL_WARNING" is introduced, and it is different from the previous one "study_tell_warning". But, the lifetime of the STUDY_TELL_WARNING_KEY for each trial is closed from the tell to the finally close, so it seems not to be a concern.)

@himkt
Copy link
Copy Markdown
Member Author

himkt commented Apr 22, 2022

Let me kindly ping @not522. 🙋

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 added code-fix Change that does not change the behavior, such as code refactoring. optuna.study Related to the `optuna.study` submodule. This is automatically labeled by github-actions. v3 Issue/PR for Optuna version 3. labels Apr 25, 2022
@not522 not522 added this to the v3.0.0-b1 milestone Apr 25, 2022
@not522 not522 merged commit 9d80368 into optuna:master Apr 25, 2022
@himkt himkt deleted the study-tell-followup branch April 25, 2022 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-fix Change that does not change the behavior, such as code refactoring. optuna.study Related to the `optuna.study` submodule. This is automatically labeled by github-actions. v3 Issue/PR for Optuna version 3.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants