Skip to content

Make plot functions return Plotly graph object.#704

Merged
toshihikoyanase merged 6 commits intooptuna:masterfrom
c-bata:update-layout-params
Nov 20, 2019
Merged

Make plot functions return Plotly graph object.#704
toshihikoyanase merged 6 commits intooptuna:masterfrom
c-bata:update-layout-params

Conversation

@c-bata
Copy link
Member

@c-bata c-bata commented Nov 14, 2019 •
edited
Loading

Summary

I want to plot intermediate values with log-scale yaxis. I once designed APIs of plotting utilities like plot_intermediate_values(study, yaxis_logscale=True) but this approach might need a lot of parameters if we want to provide more flexible options. In this PR, I added option to pass parameters into update_layout method.

Usage

Prepare a dummy study.

import optuna

study = optuna.create_study()

values = [
    [10000, 1000, 100, 10, 1, 0.1, 0.01],
    [7500, None],
    [5000, 500, 50, 5, 0.5, 0.05, 0.005],
    [6000, None],
]
    
for intermediate_values in values:
    trial_id = study._storage.create_new_trial(study.study_id)
    trial = optuna.trial.Trial(study, trial_id)

    for i, v in enumerate(intermediate_values):
        if v is None:
            study._storage.set_trial_state(trial_id, optuna.structs.TrialState.PRUNED)
            break
        trial.report(v, step=i)
    else:
        study._storage.set_trial_state(trial_id, optuna.structs.TrialState.COMPLETE)

Then plotting without params:

from optuna.visualization import plot_intermediate_values

plot_intermediate_values(study)

newplot (2)

Then plotting in log-scale yaxis:

from optuna.visualization import plot_intermediate_values

plot_intermediate_values(study, {'yaxis_type': 'log'})

newplot (1)

@c-bata
Copy link
Member Author

c-bata commented Nov 14, 2019

If you are positive to merge this, I'll add this option in other plotting functions.

@smly
Copy link
Contributor

smly commented Nov 15, 2019

Thanks for the PR. Your proposed feature seems useful!

I suggest changing the plot function plot_*() to return a plotly's figure object.
Then, this PR can resolve #601 and #602 at the same time.
Let me know your thoughts about this change. Thanks!

@c-bata
Copy link
Member Author

c-bata commented Nov 15, 2019

I suggest changing the plot function plot_*() to return a plotly's figure object.

It sounds good 👍

The only thing I'm concerned is your suggestion is a breaking change. But the problem might be relatively small because most of users called this function from jupyter.

@c-bata c-bata changed the title Add option to call plotly's update_layout method Make plot functions return Plotly graph object. Nov 16, 2019
@smly smly self-requested a review November 18, 2019 06:08
smly
smly approved these changes Nov 18, 2019
View reviewed changes
Copy link
Contributor

@smly smly 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 your comments and changes! LGTM.

I checked that the ipynb file in optuna repository does not need to be modified.
Besides, this PR resolves #601 and #602.

check2

@smly smly added this to the v0.20.0 milestone Nov 18, 2019
toshihikoyanase
toshihikoyanase requested changes Nov 20, 2019
View reviewed changes
Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

I have a comment about access to the private module of plotly. It is not introduced by your change, so please let me know if you think we should fix it in another PR.

c-bata and others added 2 commits November 20, 2019 14:25
@c-bata @toshihikoyanase
Fix references to plotly.graph_objs.Figure
a7ca725 
Co-Authored-By: Toshihiko Yanase <toshihiko.yanase@gmail.com>
@c-bata
Fix references to plotly Figure object
132697f
@c-bata
Copy link
Member Author

c-bata commented Nov 20, 2019

@toshihikoyanase Thank you for your review! I removed an import of plotly.graph_objs._figure.Figure and replace Figure with go.Figure at 132697f.

toshihikoyanase
toshihikoyanase approved these changes Nov 20, 2019
View reviewed changes
Copy link
Member

@toshihikoyanase toshihikoyanase left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

FYI, I confirmed that this change resolved your example task as expected.

image
image

@toshihikoyanase toshihikoyanase merged commit 33cda45 into optuna:master Nov 20, 2019
@c-bata c-bata deleted the update-layout-params branch November 20, 2019 06:19
@toshihikoyanase toshihikoyanase added the compatibility Change that breaks compatibility. label Nov 20, 2019
@hvy hvy mentioned this pull request Nov 20, 2019
Closed
@toshihikoyanase toshihikoyanase mentioned this pull request Dec 11, 2019
Merged
@c-bata c-bata mentioned this pull request Apr 28, 2020
Merged
@c-bata c-bata mentioned this pull request Aug 30, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@toshihikoyanase toshihikoyanase toshihikoyanase approved these changes

+1 more reviewer

@smly smly smly approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

compatibility Change that breaks compatibility.

Projects

None yet

Milestone

v1.0.0

Development

Successfully merging this pull request may close these issues.

3 participants

@c-bata @smly @toshihikoyanase