Add rank plot #4427
Add rank plot #4427
Conversation
github-actions
bot
added
the
optuna.visualization
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 #4427 +/- ##
==========================================
+ Coverage 90.79% 90.89% +0.10%
==========================================
Files 187 188 +1
Lines 13932 14132 +200
==========================================
+ Hits 12649 12845 +196
- Misses 1283 1287 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@cross32768 Could you review this PR? |
|
@hvy Could you review this PR? |
There was a problem hiding this comment.
Thanks for the PR.
I've mostly only went over the data preparation part but left some comments if you could take a look.
Also,
- Could you add unit tests similar to our other visualization functions and the logic indeed has several conditionals?
- Could you add this function to the API reference under
docs/? - Could you import this function in
visualization/__init__.py? - How about making the plots consistent when plotting with a single parameter specified using
params=["some_param"]and the diagonal plots created byparams=None?
This is more of a question but did you consider add a thin border around the scatter dots as in plot_slice? Applying it here as well might be consistent and perhaps make it easier to comprehend? 🤔
|
This pull request has not seen any recent activity. |
|
We noticed that this PR requires |
I tried it but it produced no visible difference to me. I can add it to be consistent. |
|
@hvy Thanks for your comments. I fixed those points, could you take another look? |
contramundum53
added
the
enhancement
Hm, I noticed a visual difference with |
| @@ -71,7 +71,7 @@ jobs: | |||
| # Install dependencies with minimum versions. | |||
| pip uninstall -y alembic cmaes packaging sqlalchemy plotly scikit-learn | |||
| pip install alembic==1.5.0 cmaes==0.9.1 packaging==20.0 sqlalchemy==1.3.0 numpy==1.20.3 tqdm==4.27.0 colorlog==0.3 PyYAML==5.1 | |||
| pip install plotly==4.9.0 scikit-learn==0.24.2 # optional extras | |||
| pip install plotly==5.0.0 scikit-learn==0.24.2 # optional extras | |||
There was a problem hiding this comment.
should we check this with the other maintainers? also, should we not update pyproject.toml ?
There was a problem hiding this comment.
Updating pyproject.toml would prevent plotly-4.9.0 users from using other visualization tools or accessing other optuna updates, so I don't think that's desirable.
For whether to depend on the newer version of plotly or reimplment the logic ourselves, I think we could have a discussion. What do you think, @toshihikoyanase ?
There was a problem hiding this comment.
I had a discussion with @toshihikoyanase . We agreed that
- Updating the required dependency would be undesirable.
- Since this functionality is experimental, we could add a nice assertion message on
plotlyversion and reimplement the logic if users really need it.
There was a problem hiding this comment.
I think we may want to memorize why plotly versions are inconsistent between pyproject.toml and this configuration. Someone may try to fix the inconsistency. How about leaving a comment on it?
There was a problem hiding this comment.
I added a comment.
Could you show me some images? I'm not sure if I'm reproducing it right. @hvy |
|
@not522 Could you review this PR? |
|
Thank you for responding to the review. LGTM! |
|
This pull request has not seen any recent activity. |
| showscale=True, | ||
| cmin=0, | ||
| cmax=1, | ||
| colorbar=dict(thickness=10, tickvals=tick_coloridxs, ticktext=ticktext), |
There was a problem hiding this comment.
How about adding the title?
| colorbar=dict(thickness=10, tickvals=tick_coloridxs, ticktext=ticktext), | |
| colorbar=dict(thickness=10, tickvals=tick_coloridxs, ticktext=ticktext, title=f"Rank ({info.target_name})"), |
There was a problem hiding this comment.
In my opinion, this looks a bit too redundant and noisy.
There was a problem hiding this comment.
Hmmm, but without that, isn't it hard to understand what the numbers mean?
There was a problem hiding this comment.
What about changing the title of the plot to Rank (Objective Value)?
Motivation
optuna/optuna-dashboard#384
Description of the changes
Add
plot_rankfunction.This plots the parameter relationship as pairwise scatter plots with colors indicating the (normalized) rank of the objective value within the entire study.