search

INRIA / scikit-learn-mooc

Machine learning in Python with scikit-learn MOOC
https://inria.github.io/scikit-learn-mooc
Creative Commons Attribution 4.0 International
1.12k stars 516 forks source link

ENH use probability estimates in DecisionBoundaryDisplay #710

Closed glemaitre closed 1 year ago

glemaitre commented 1 year ago

Fixes #435.

It shows different levels of probabilities instead of the deterministic prediction.

ogrisel commented 1 year ago

The git history is very weird on this PR. The diff looks good though. I will try to see if a rebase of cherry-pick can clean up the history.

I will force push the result.

ogrisel commented 1 year ago

Done. Let me expand a bit on the interpretation of those plots.

ogrisel commented 1 year ago

@ArturoAmorQ @glemaitre I pushed my analysis of the updated probability estimates.

glemaitre commented 1 year ago

I am approving my own PR :). I am not entirely sure about my proposal of switching "dark" to "opaque"/"transparent". Feel free to ignore it if you think that it does not bring any value.

ogrisel commented 1 year ago

I pushed an extension in anticipation of #711.

ArturoAmorQ commented 1 year ago

I'm not really sure about adding a new concept inside an exercise. But what worries me the most is that introducing too much information may be distracting from the main point of the exercise, which is seeing the effect of regularization on the coefficients.

As mentioned in this comment: predict_proba is introduced in a more natural way in the Build a classification decision tree notebook in M5. If we really want to introduce the concept on predict_proba for the DecisionBoundaryDisplay, we should do it after such notebook.

Edit: I opened #722 to fast-forward the introduction of predict_proba.

ogrisel commented 1 year ago

But I think this is important to show that regularization has also an impact on the certainty of the linear classifier.

This cannot be introduced in a notebook about decision trees.

I think it's fine to introduce some new concept in ungraded exercises. It makes the exercises more interesting by being less redundant (not just a repetition) of the previous notebooks.

Furthermore I took care to phrase the question is in a way that is just "what do you observe", not "why do you observe what you observe".

ArturoAmorQ commented 1 year ago

I just noticed this notebook starts with

The parameter penalty can control the type of regularization to use [...]

This is a relic from when the notebook Linear model for classification introduced the concept of penalty. Now that it is no longer the case, I think we should modify the intro to something similar to:

The scikit-learn implementation of LogisticRegression has a hyperparameter penalty that can control the type of regularization to use [...]

ArturoAmorQ commented 1 year ago

By the way, I opened #715 to modify the introductory notebook Linear model for classification so that it mentions predict_proba as a call for action to those interested. Maybe we can do something similar with this PR. The message should be something similar to:

Do the same plots with response_method="predict" as done in the Linear model for classification notebook with different values for C. If you feel like going further, try changing to response_method="predict_proba" to see that regularization has also an impact on the certainty of the linear classifier.

ogrisel commented 1 year ago

I update the intro paragraph to address https://github.com/INRIA/scikit-learn-mooc/pull/710#issuecomment-1715359929. Any further comments @ArturoAmorQ?

ArturoAmorQ commented 1 year ago

I re-organized the exercise in the last commit (sorry for the not-descriptive at all commit name). Opinions on my refactoring are very welcomed.

ArturoAmorQ commented 1 year ago

@ogrisel I addressed the rest of your comments on 438adbb. Still I would like a review on these changes to be completely sure the message is correct.