Skip to content

ENH Allows plotting max class for multiclass in DecisionBoundaryDisplay #29797

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 62 commits into from
Mar 7, 2025
Merged

ENH Allows plotting max class for multiclass in DecisionBoundaryDisplay #29797

merged 62 commits into from
Mar 7, 2025

Conversation

lucyleeow
Copy link
Member

Reference Issues/PRs

Towards #27462

What does this implement/fix? Explain your changes.

Allows DecisionBoundaryDisplay to represent all classes for multiclass decision_function and predict_probas by plotting the class with the max response at each point.

Have closely followed the code here: #27291 (comment)

Not 100% on what the colour API should be, open to change.

contour and contourf have a colors parameter, which can be used instead of cmap BUT pcolormesh only has cmap. For simplicity, I've decided to only allow users to pass cmap. This probably makes more sense in this context vs a list of colors to cycle through.

Any other comments?

WIP - need to add tests once we're happy with plot appearance and API

@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 6, 2024 •
edited
Loading

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: 5dc5b7b. Link to the linter CI: here

@glemaitre glemaitre self-requested a review September 7, 2024 20:51
glemaitre
glemaitre reviewed Sep 12, 2024
View reviewed changes
@lucyleeow
Copy link
Member Author

lucyleeow commented Sep 13, 2024 •
edited
Loading

I'm also going to try amend example: Logistic Regression 3-class Classifier to show a plot using this enhancement. Though as we use the same X we used to fit the estimator, the plot probably won't look great.
I could instead use Nystroem, though this would only be for the purpose of showing a prettier plot...WDYT @glemaitre ?

@glemaitre
Copy link
Member

I would almost argue that we be removing the LogisticRegression plot because it does not have that much values :).

I'm thinking that we could instead modify this one: https://scikit-learn.org/stable/auto_examples/classification/plot_classification_probability.html#sphx-glr-auto-examples-classification-plot-classification-probability-py and add an additional column to the plot with the max. Like this we could remove the OvR LR (it is deprecated), and instead use a Nystroem with LR.

glemaitre
glemaitre reviewed Sep 13, 2024
View reviewed changes
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

Ups I see I didn't post some review that I had. This should be partial comments.

@lucyleeow lucyleeow changed the title WIP ENH Allows plotting max class for multiclass in DecisionBoundaryDisplay ENH Allows plotting max class for multiclass in DecisionBoundaryDisplay Sep 14, 2024
@lucyleeow
Copy link
Member Author

I'm thinking that we could instead modify this one: https://scikit-learn.org/stable/auto_examples/classification/plot_classification_probability.html#sphx-glr-auto-examples-classification-plot-classification-probability-py and add an additional column to the plot with the max. Like this we could remove the OvR LR (it is deprecated), and instead use a Nystroem with LR.

Do you want me to do this in this PR or a separate PR (since removing OvR LR may technically not be considered part of this work)?

@glemaitre
Copy link
Member

Do you want me to do this in this PR or a separate PR (since removing OvR LR may technically not be considered part of this work)?

We can do it in this PR and limit the change to this single example.

@glemaitre glemaitre self-requested a review November 15, 2024 07:46
@glemaitre
Copy link
Member

Cool. I will have a look now.

glemaitre
glemaitre approved these changes Nov 15, 2024
View reviewed changes
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM

@lucyleeow lucyleeow added the Waiting for Second Reviewer First reviewer is done, need a second one! label Nov 15, 2024
@glemaitre
Copy link
Member

Maybe @ogrisel could have a look since we discussed that feature some time ago.

@lucyleeow
Copy link
Member Author

@ogrisel gentle ping about this 😬 Thank you!

@ogrisel
Copy link
Member

ogrisel commented Mar 6, 2025 •
edited
Loading

@lucyleeow I pushed two commits of changes I wanted to do while reviewing the PR:

  • the first selects a qualitative colormap automatically and adjusts it to the observed number of classes;
  • the second is a refactoring of the example to show how to read those plots to draw conclusions on the impact of models parameters on the shape of the decision boundary and the magnitude of the predicted probabilities.

I will do a full review tomorrow but this LGTM.

ogrisel
ogrisel reviewed Mar 6, 2025
View reviewed changes
examples/classification/plot_classification_probability.py
# plot the probability estimate provided by the classifier
disp = DecisionBoundaryDisplay.from_estimator(
classifier,
X,
X_train,
response_method="predict_proba",
Copy link
Member

@ogrisel ogrisel Mar 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

This is a pitfall of this method. By default, it would use response_method="decision_function" which is much harder to interpret in my opinion (especially when comparing different model classes).

I think we should change the "auto" policy to use favor predict_proba when available but this should rather be done in a dedicated follow-up PR.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, if we use response_method="predict_proba", maybe the vmin=0 and vmax=1 parameters could be set automatically to make the use DecisionBoundaryDisplay terser.

Copy link
Member

Choose a reason for hiding this comment

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

It looks indeed like a good suggestion.

@lucyleeow
Copy link
Member Author

FYI @ogrisel I'm just going to fix the merge conflicts

@ogrisel ogrisel enabled auto-merge (squash) March 7, 2025 08:30
@ogrisel ogrisel merged commit 88283ee into scikit-learn:main Mar 7, 2025
31 checks passed
@ogrisel
Copy link
Member

ogrisel commented Mar 7, 2025

Thanks @lucyleeow!

@lucyleeow lucyleeow deleted the dbplot_multi branch March 7, 2025 10:14
@lucyleeow
Copy link
Member Author

Thanks for finishing this one off @ogrisel !

@glemaitre
Copy link
Member

Thanks @ogrisel

@ogrisel
Copy link
Member

ogrisel commented Mar 7, 2025

@lucyleeow If you are looking for follow-up improvements, please consider those two comments:

#29797 (comment)

@lesteve
Copy link
Member

lesteve commented Mar 10, 2025

FYI this broke the CI when matplotlib is not installed, fix in #30971

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
module:inspection Waiting for Second Reviewer First reviewer is done, need a second one!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@lucyleeow @glemaitre @ogrisel @lesteve