ENH Add libsvm-like calibration procedure to CalibratedClassifierCV #17856

lucyleeow · 2020-07-07T15:59:44Z

Reference Issues/PRs

closes #16145
Follows from PR #16167
The first step was to refactor CalibratedClassifierCV, which was done in #17803. This is a branch off of #17803. (will close #17803)

What does this implement/fix? Explain your changes.

Add ensemble option to CalibratedClassifierCV.
ensemble=True would be the default and would be the current implementation.
ensemble=False would be the same as the libsvm implementation (used when probability=True):

When cv is not 'prefit':

Concatenate predictions from each cv. Use all predictions to train calibrator.
Classifiers fit after each cv would be destroyed
Use classifier fit on all data at prediction time

Note ensemble=False is the procedure described in the original Platt paper (see #16145 (comment))

Any other comments?

lucyleeow · 2020-07-07T16:36:21Z

@NicolasHug, this is probably not going to be a problem but I thought I would mention it.

We use cross_val_predict to get the predictions, where if a training subset is missing 1 or more classes, it is replaced by 0 if predict_proba otherwise by ('we approximate negative infinity') 'the minimum finite float value for the dtype' for decision_function.
CalibratedClassifierCV uses 0 for both predict_proba and decision_function.

NicolasHug

Thanks @lucyleeow , made a first pass.

I think the refactoring makes sense, WDYT?

I haven't looked in detail at the changes related to the label encoders though. Are these needed here or can they wait for another PR?

sklearn/calibration.py

NicolasHug · 2020-07-07T17:57:20Z

sklearn/calibration.py

+                # When binary, proba of clf_fitted.classes_[1]
+                # output but `pos_class_indices` = 0


it seems that the grammar is incorrect?

I amended to:

# When binary, df consists of predictions for # clf_fitted.classes_[1] but `pos_class_indices` = 0

not sure if this is clearer

NicolasHug · 2020-07-07T18:03:11Z

sklearn/calibration.py

+                if base_estimator_method == "decision_function":
+                    if df.ndim == 1:
+                        df = df[:, np.newaxis]
+                else:
+                    if len(self.label_encoder_.classes_) == 2:
+                        df = df[:, 1:]


The duplication is a bit unfortunate, there should be a way to merge this logic with that of _get_predictions

Maybe by extracting this into a _reshape_df utility?
Or maybe we can even let _get_predictions call cross_val_predict somehow, so that we can also merge the method checks above.
Also is df = df[:, 1:] just equivalent to df = df[:, 1]?

Or maybe we can even let _get_predictions call cross_val_predict somehow, so that we can also merge the method checks above.

This is what I was trying to do before - but _get_predictions is is used within _fit_calibrator which is within the cv for loop. This is why I wanted to put the cv for loop within _get_predictions but it got complicated.

I ~~will~~ can add a _reshape_df function but I have a little dislike for the df naming as it is not always 'decision_function`. (I also first think df=dataframe but maybe that is just me) WDYT?
(of course if this naming is used elsewhere, happy to keep)

Also I can't think of an elegant way to add the _reshape_df function as we get the predictions of the classifier in _get_predictions but we use cross_val_predict to get the predictions in the case above...

Yes I was confused by df too. We can rename it as predictions or just preds.

Also I can't think of an elegant way to add the _reshape_df

Would it help to also add a _get_method helper, like

def _get_method(clf): # return decision_function (the actual method) or predict_proba or raise error

?

Looking into it, _fit_calibrator returns a _CalibratedClassifierPipeline instance for each cv fold. For ensemble=False, this won't work (for how _fit_calibrator works atm) - we want to fit once with all the predictions. We would need to take _fit_calibrator out of the cv for loop.

Looking into it, _fit_calibrator returns a _CalibratedClassifierPipeline instance for each cv fold

Only when ensemble=True, right?

There's no CV loop when ensemble=False, the "loop" is in cross_val_predict and we only need to call _fit_calibrator once. Maybe I misunderstood?

This is difficult over text! I may be confused too.

At the moment we have:

class CalibratedClassifierCV(): def fit(): if self.cv == "prefit": # `_fit_calibrator` using all X and y # append one `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_` else: if ensemble: for train, test in cv.split(X, y): # fit classifier # call `_fit_calibrator`, producing a `_CalibratedClassiferPipeline` instance # append `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_`, # ending up with `n_cv` `_CalibratedClassiferPipeline` instances else: # Use `cross_val_predict` to get all `preds` # call `_fit_calibrator` using `preds` and y # append one `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_`

we want to get rid of the last else as there is duplication of code there. With the new _get_prediction_method we have:

class CalibratedClassifierCV(): def fit(): if self.cv == "prefit": # `_fit_calibrator` using all X and y # append one `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_` else: for train, test in cv.split(X, y): # fit classifier # call `_fit_calibrator`, which first calls `_get_predictions`. # In case of `ensemble=False`, we use use `partial(cross_val_predict())` (this works fine) # `_fit_calibrator` also fits calibrators and produces `_CalibratedClassiferPipeline` instance # Only one _CalibratedClassiferPipeline` is required when `ensemble=False` # (i.e. shouldn't be in the for loop) # when `ensemble=True` `n_cv` `_CalibratedClassiferPipeline` instances are needed # (i.e., should be in the for loop)

Sorry if I wasn't clear! I don't think we can move away from the first version (with the last else). The duplication of code that I wanted to avoid was:

getting the right method

if hasattr(base_estimator, "decision_function"): base_estimator_method = "decision_function" elif hasattr(base_estimator, "predict_proba"): base_estimator_method = "predict_proba"

reshaping the predictions

if base_estimator_method == "decision_function": if preds.ndim == 1: preds = preds[:, np.newaxis] else: if len(self.label_encoder_.classes_) == 2: preds = preds[:, 1]

Right now, these 2 things happen both in CalibratedClassifierCV.fit (for ensemble=False) and in _get_predictions (for ensemble=True).

So to expand on my previous suggestion:

def get_prediction_method(clf, return_string=False): # return clf.decision_function or clf.predict_proba or their corresponding string # we need strings when using cross_val_predict def _get_predictions(X, pred_func): # pred_func is either a method of the clf, or a partial cross_val_predict preds = pred_func(X) # reshape preds here return preds class CalibratedClassifierCV(): def fit(): if self.cv == "prefit": # `_fit_calibrator` using all X and y # append one `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_` else: if ensemble: for train, test in cv.split(X, y): # fit classifier on train preds = _get_predictions(test, get_prediction_method(clf, return_string=False) # call `_fit_calibrator`, producing a `_CalibratedClassiferPipeline` instance # append `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_`, # ending up with `n_cv` `_CalibratedClassiferPipeline` instances else: preds = _get_predictions(X, partial(cross_val_predict(..., method=get_prediction_method(clf, return_string=True))) # call `_fit_calibrator` using `preds` and y # append one `_CalibratedClassiferPipeline` instance to `self.calibrated_classifiers_`

I think we might need to modify _fit_calibrator to only accept preds and not call _get_predictions

FYI:

Also is df = df[:, 1:] just equivalent to df = df[:, 1]?

it seems that it is not the same, you get 2d output with df[:, 1:]

lucyleeow · 2020-07-08T11:54:43Z

I haven't looked in detail at the changes related to the label encoders though. Are these needed here or can they wait for another PR?

There wasn't a big change to this. Previously we passed the classes to _CalibratedClassifier:

scikit-learn/sklearn/calibration.py

Lines 180 to 181 in fd23727

    
           calibrated_classifier = _CalibratedClassifier( 
        
               this_estimator, method=self.method, classes=self.classes_)

and then instantiate a new LabelEncoder in _CalibratedClassifier.fit:

scikit-learn/sklearn/calibration.py

Lines 325 to 329 in fd23727

    
           self.label_encoder_ = LabelEncoder() 
        
           if self.classes is None: 
        
               self.label_encoder_.fit(y) 
        
           else: 
        
               self.label_encoder_.fit(self.classes)

(note: self.classes is never None when using CalibratedClassifierCV)

I just changed it so we do not pass the classes but the fitted LabelEncoder instance. I don't think it is a big change but I can change it back.

lucyleeow · 2020-07-09T19:13:46Z

I've make the changes. I like that _get_predictions and _fit_calibrator are separate now. Not sure if it's overall better (the ensemble=False section is a bit messy), will sleep on it. Also, is it appropriate to use method.__name__, to get the str of the method?

I do think that it is a bit of duplication to have both _check_classifier_response_method and get_prediction_method. I think a single utility fun like:

get_prediction_method(estimator, method, allow_alt=False):
    """Return prediction method of `estimator`.

    Parameters
    ----------------
    estimator : Estimator instance

    method : {'decision_function', 'predict_proba'}

    allow_alt : bool, default=False
        Whether to return the alternative method, if `method` does not exist for `estimator`.

would do the trick. But maybe that is a separate issue.

NicolasHug

Thanks @lucyleeow , made another pass.

I think the use of partial is appropriate and the refactoring looks good to me. We'll need tests for the new ensemble parameter ;)

I do think that it is a bit of duplication to have both _check_classifier_response_method and get_prediction_method [...]

Yes we can consider having a unified utility, but indeed this would be better done in a separate PR.

sklearn/calibration.py

lucyleeow · 2020-10-08T21:58:25Z

ping @NicolasHug @ogrisel
It would be great to get this in. As there is such extensive change to calibration.py, merge conflicts are confusing to fix, e.g., caused by #18332

ogrisel

This looks very good to me. Thank you very much @lucyleeow for your patience.

Here are some fixes / small suggestions for further improvements but otherwise it seems ready for second review and merge.

Please add a new entry to doc/whats_new/0.24.rst.

doc/modules/calibration.rst

sklearn/calibration.py

doc/modules/calibration.rst

sklearn/calibration.py

sklearn/tests/test_calibration.py

sklearn/calibration.py

lucyleeow · 2020-10-17T07:11:01Z

Thanks for the review @ogrisel, suggestions added!

doc/whats_new/v0.24.rst

…lbclf_ensemble

glemaitre

It looks good. Just a couple of nitpicks.

sklearn/tests/test_calibration.py

ogrisel · 2020-10-29T17:51:18Z

Merged! Thank you very much @lucyleeow!

ogrisel · 2020-10-29T18:02:29Z

sklearn/tests/test_calibration.py

                                       sparse.csr_matrix(X_test))]:
        cal_clf = CalibratedClassifierCV(
-            clf, method=method, cv=2, ensemble=ensemble
+            clf, method=method, cv=5, ensemble=ensemble


I used cv=5 whenever we assess the quality of the calibration because cv=2 trains models on much small dataset and can degrade the base model performance, hence make the test quite brittle.

lucyleeow added 11 commits June 24, 2020 17:08

wip

5b248de

refactor

ccf55fb

merge master

1f97a53

clean up, tests pass

2b87d0b

remove debugging test

2ee24cd

lint

4ae878b

suggestions

12cee4d

check class y

e183131

add att docstr

1b411f7

wip

b724760

first iter, tests pass

dadefa7

lucyleeow mentioned this pull request Jul 7, 2020

Refactor CalibratedClassifierCV #17803

Closed

lint

28fc1c0

NicolasHug reviewed Jul 7, 2020

View reviewed changes

lucyleeow added 5 commits July 8, 2020 17:19

rename df to preds

443ae50

lint

093105d

lint

7dcd4e5

sep pred and fit calib

9e1ea37

lint

42d405b

use partial

b628835

NicolasHug reviewed Jul 27, 2020

View reviewed changes

lucyleeow added 6 commits July 28, 2020 17:16

suggestions, update docstring

6d30cf8

merge master, amend for joblib

062e804

lint

fec2395

Merge branch 'master' into calbclf_ensemble

6c916a7

add tests

6701bc7

lint

82b885f

lucyleeow added the Waiting for Reviewer label Oct 8, 2020

ogrisel self-requested a review October 15, 2020 05:44

ogrisel approved these changes Oct 15, 2020

View reviewed changes

ogrisel added this to the 0.24 milestone Oct 15, 2020

lucyleeow added 2 commits October 17, 2020 19:18

suggestions, whats new

1b2e3a1

expand benefit

d396c6e

formatting

8c07fd4

ogrisel requested review from NicolasHug and glemaitre October 20, 2020 11:13

ogrisel reviewed Oct 20, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

ogrisel reviewed Oct 20, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

suggestion

577c7b3

NicolasHug reviewed Oct 21, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

lucyleeow and others added 2 commits October 22, 2020 13:15

add user

bfaf3e1

Merge branch 'master' of github.com:scikit-learn/scikit-learn into ca…

cb83a96

…lbclf_ensemble

glemaitre approved these changes Oct 28, 2020

View reviewed changes

suggestions

52c821c

glemaitre self-assigned this Oct 29, 2020

ogrisel added 2 commits October 29, 2020 18:20

More stable multiclass test with Brier score

deb75fc

Unused import

aff6bb8

ogrisel merged commit 8471c83 into scikit-learn:master Oct 29, 2020

ogrisel reviewed Oct 29, 2020

View reviewed changes

glemaitre mentioned this pull request Oct 29, 2020

Issue with CalibratedClassifierCV with multiclass classification problems #18709

Open

ogrisel mentioned this pull request Oct 30, 2020

DOC improve n_jobs docs in CalibratedClassifierCV #18633

Closed

lucyleeow removed the Waiting for Reviewer label Jan 26, 2021

lucyleeow deleted the calbclf_ensemble branch January 26, 2021 07:53

cfauchereau mentioned this pull request May 13, 2021

FIX Fix error when using Calibrated with Voting #20087

Merged

		# When binary, proba of clf_fitted.classes_[1]
		# output but `pos_class_indices` = 0

Uh oh!

ENH Add libsvm-like calibration procedure to CalibratedClassifierCV #17856

ENH Add libsvm-like calibration procedure to CalibratedClassifierCV #17856

Uh oh!

Conversation

lucyleeow commented Jul 7, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

lucyleeow commented Jul 7, 2020

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jul 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucyleeow Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucyleeow commented Jul 8, 2020

Uh oh!

lucyleeow commented Jul 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lucyleeow commented Oct 8, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ogrisel left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lucyleeow commented Jul 7, 2020 •

edited

Loading

lucyleeow Jul 8, 2020 •

edited

Loading

lucyleeow Jul 9, 2020 •

edited

Loading

lucyleeow commented Jul 9, 2020 •

edited

Loading

lucyleeow commented Oct 8, 2020 •

edited

Loading