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
[hotfix] Fix the bug of matplotlib's plot_rank function #5133
Conversation
c98c5d5
to
a30cc9f
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5133 +/- ##
==========================================
- Coverage 89.43% 89.41% -0.02%
==========================================
Files 205 205
Lines 15151 15160 +9
==========================================
+ Hits 13550 13556 +6
- Misses 1601 1604 +3 ☔ View full report in Codecov by Sentry. |
a30cc9f
to
9483a74
Compare
@eukaryo @nabenabe0928 Could you review this PR? |
The windows CI will be resolved by #5135 . |
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.
Hi thank you for the quick fix!
I checked the color map array for matplotlib and plotly just in case with the following code:
import plotly
import matplotlib.pyplot as plt
import numpy as np
def get_color_scale_in_plotly(color_idxs: np.ndarray) -> np.ndarray:
colormap = "RdYlBu_r"
# sample_colorscale requires plotly >= 5.0.0.
labeled_colors = plotly.colors.sample_colorscale(colormap, color_idxs)
scaled_rgb_colors = np.array([plotly.colors.unlabel_rgb(cl) for cl in labeled_colors])
return scaled_rgb_colors
def get_color_scale_in_matplotlib(color_idxs: np.ndarray) -> np.ndarray:
colormap = "RdYlBu_r"
cmap = plt.get_cmap(colormap)
colors = cmap(color_idxs)[:, :3]
rgb_colors = np.asarray(colors * 256, dtype=int)
return rgb_colors
color_idxs = np.linspace(0, 1, 101)
cmap_plotly = get_color_scale_in_plotly(color_idxs)
cmap_matplotlib = get_color_scale_in_matplotlib(color_idxs)
Although they are not perfectly identical, I suppose the difference does not really matter from the user side, so I will leave it intact!
Thank you for your review. PTAL. |
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 the 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.
The code looks good to me. Let me check the behavior.
The code works perfect. LGTM! import optuna
study = optuna.create_study()
def objective(trial):
x = trial.suggest_uniform('x', -10, 10)
y = trial.suggest_uniform('y', -10, 10)
return (x - 2) ** 2 + (y + 3) ** 2
study.optimize(objective, n_trials=100)
import optuna.visualization.matplotlib
optuna.visualization.matplotlib.plot_rank(study) |
@contramundum53 @nabenabe0928 I've just noticed a minor bug where the RGB values could potentially reach 256, though they should be within the range [0, 255]. Could you please take a look at the following change? |
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.
I checked the change!
LGTM! |
Motivation
Fix #5115
Description of the changes
Fix the bug of matplotlib's
plot_rank()
function.