Conversation
|
Should we change |
Codecov Report
@@ Coverage Diff @@
## master #3348 +/- ##
==========================================
- Coverage 91.53% 91.53% -0.01%
==========================================
Files 158 158
Lines 12184 12193 +9
==========================================
+ Hits 11153 11161 +8
- Misses 1031 1032 +1
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
|
Thanks for the PR. |
|
@belldandyxtq Please apply the |
|
This pull request has not seen any recent activity. |
|
note: we need to write a migration script. |
HideakiImamura
left a comment
There was a problem hiding this comment.
Thanks for the mob review! LGTM.
[TODO] We need to add a migration script. I must be done until the next release, but it should be ASAP.
|
I fixed the conflict. @himkt Please merge this PR after your approval. |
himkt
left a comment
There was a problem hiding this comment.
LGTM.
Note that after this PR being merged, trial.report(step=x, value=float("nan")) does not raise an exception without writing an intermediate entry until a storage migration. The current behavior is that stops an optimization with raising an exception. Users who stops the optimization using trial.report + value=nan may observe this behavior change (but we think no one rely on this behavior, since pruners consider trials to be pruned when nan exists in intermediates).
Motivation
This is a part of #3205 to support
nanintrial.report. As shown in the issue,trial.reportfails when the backend isMySQL, sinceMySQLcannot storenan.Description of the changes
nantoNoneto store in the backend.naninreport.