-
-
Notifications
You must be signed in to change notification settings - Fork 971
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
Add timeline plot with plotly as backend #4470
Conversation
Thank you for your PR! Let me check it. |
I see. I have added a figure of timeline plot to the description above. Here is sample code to generate the figure, which is basically the same code as in the docstring of
|
There was a problem hiding this 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. I have several comments. Please take a look.
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## master #4470 +/- ##
==========================================
+ Coverage 90.32% 90.35% +0.02%
==========================================
Files 183 184 +1
Lines 14142 14191 +49
==========================================
+ Hits 12774 12822 +48
- Misses 1368 1369 +1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your PR and example. Let me share some comments on the implementation of the timeline plot.
Next, I'll check the test cases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me share my comments on the tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your update. Overall, it looks almost good to me, but I have some additional comments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update. Basically, LGTM. I have 2 minor comments. PTAL.
Co-authored-by: Toshihiko Yanase <toshihiko.yanase@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your update. LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update! LGTM!
Motivation
To provide an easy way to check the execution status, I want to implement a function to plot the Trials as a timeline.
The timeline here is a Gantt chart with time on the horizontal axis and trial number on the vertical axis, which makes it clear at a glance whether trials are being executed in parallel and whether there are any trials with abnormally long execution times.
Description of the changes
Add timeline plot using plotly's horizontal bar graph in
optuna/visualization/_timeline.py
.Add Unit tests in
tests/visualization_tests/test_timeline.py
.