ENH Adds multimetric support to check_scoring by thomasjpfan · Pull Request #28360 · scikit-learn/scikit-learn

thomasjpfan · 2024-02-04T21:21:43Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

This PR adds multi-metric support to check_scoring. This gives a public inference for returning a multiple metric scoring that uses the caching from scoring. With this PR, one can write the following to get a multi-metric scorer:

mutli_scoring = check_scoring(scoring=["r2", "roc_auc", "accuracy"])

Any other comments?

There are more places that can use this, but it requires #28359 to be merged in first.

github-actions · 2024-02-04T21:24:05Z

✔️ Linting Passed

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

_{Generated for commit: 867cc10. Link to the linter CI: here}

eddiebergman

Looks good. One place that also seems to do something similiar to create multiple scorers is BaseSearchCV

scikit-learn/sklearn/model_selection/_search.py

Lines 784 to 815 in bb87768

    
               def _get_scorers(self, convert_multimetric): 
        
                   """Get the scorer(s) to be used. 
        
                   This is used in ``fit`` and ``get_metadata_routing``. 
        
                   Parameters 
        
                   ---------- 
        
                   convert_multimetric : bool 
        
                       Whether to convert a dict of scorers to a _MultimetricScorer. This 
        
                       is used in ``get_metadata_routing`` to include the routing info for 
        
                       multiple scorers. 
        
                   Returns 
        
                   ------- 
        
                   scorers, refit_metric 
        
                   """ 
        
                   refit_metric = "score" 
        
                   if callable(self.scoring): 
        
                       scorers = self.scoring 
        
                   elif self.scoring is None or isinstance(self.scoring, str): 
        
                       scorers = check_scoring(self.estimator, self.scoring) 
        
                   else: 
        
                       scorers = _check_multimetric_scoring(self.estimator, self.scoring) 
        
                       self._check_refit_for_multimetric(scorers) 
        
                       refit_metric = self.refit 
        
                       if convert_multimetric and isinstance(scorers, dict): 
        
                           scorers = _MultimetricScorer( 
        
                               scorers=scorers, raise_exc=(self.error_score == "raise") 
        
                           ) 
        
                   return scorers, refit_metric

eddiebergman · 2024-02-05T10:19:29Z

sklearn/metrics/_scorer.py

        return get_scorer(scoring)
+    if isinstance(scoring, (list, tuple, set, dict)):
+        scorers = _check_multimetric_scoring(estimator, scoring=scoring)
+        return _MultimetricScorer(scorers=scorers)


One thing that is occluded is the raise_exc: bool = True argument of _MultimetricScorer. I have personally never used it so I do not mind. Is the decision not to support passing it in?

I did not want to increase the scope of this PR. Usually increasing the scope makes it harder to merge.

If we want to add raise_exc, then it can be done in a follow up PR.

eddiebergman · 2024-02-05T10:23:25Z

sklearn/metrics/_scorer.py

+
+        - a list or tuple of unique strings;
+        - a callable returning a dictionary where the keys are the metric
+          names and the values are the metric scores;


Suggested change

names and the values are the metric scores;

names and the values are the metric scorers;

thomasjpfan · 2024-02-05T17:11:01Z

One place that also seems to do something similiar to create multiple scorers is BaseSearchCV

Changing that part would increase the scope of this PR. #28359 is a step toward in refactoring _get_scorers, independent of this PR.

Once #28359 is merged, then we can have a follow up PR to refactor _get_scorers.

adrinjalali

I think we should make _MultimetricScorer public when we're returning it in a public function. Otheriwise lgtm.

thomasjpfan · 2024-02-07T15:48:02Z

I think we should make _MultimetricScorer public when we're returning it in a public function. Otheriwise lgtm.

For the public API, check_scoring returns a callable and _MultimetricScorer is an implementation detail. We already do this with make_scorer, which also returns a private scorer class, but the public API only promises a callable.

glemaitre · 2024-02-08T18:41:49Z

For the public API, check_scoring returns a callable and _MultimetricScorer is an implementation detail. We already do this with make_scorer, which also returns a private scorer class, but the public API only promises a callable.

I agree with this statement. However, I would implement two additional things.

First, having a nicer __repr__ to be less surprising while manipulating a private object. This is currently the difference between the _Scorer and _MultiScorer:

In [5]: check_scoring(estimator=LogisticRegression(), scoring=scoring)
Out[5]: <sklearn.metrics._scorer._MultimetricScorer at 0x11c316830>

In [6]: check_scoring(estimator=LogisticRegression(), scoring=scoring["accuracy"])
Out[6]: make_scorer(accuracy_score, response_method='predict')

Then, I think that we should have an entry point in the user guide: https://scikit-learn.org/dev/modules/model_evaluation.html#using-multiple-metric-evaluation. We could have a call to check_scoring there and a small description.

eddiebergman · 2024-02-08T20:25:12Z

@glemaitre side question, is there any scope to make multimetric scorers publicly available through the get_scorer function? It's already the defacto standard for getting a scorer and check_scoring doesn't seem like an intuitive name to get a multi metric scorer.

glemaitre · 2024-02-08T21:12:57Z

It's already the defacto standard for getting a scorer and check_scoring doesn't seem like an intuitive name to get a multi metric scorer.

Yep check_scoring is more a third-party library way to validate a container of scoring. And actually my remark regarding the documentation should be more towards this intent of documenting a developer tool.

I think that we need to make sure that we can pass multiple scorer in the scoring parameter everywhere in scikit-learn before to expose it through get_scorer. But I don't see a reason why not at a first glance.

adrinjalali

Nice!

glemaitre

LGTM. Thanks @thomasjpfan

ENH Adds multimetric support to check_scoring

9a57fe0

github-actions bot added module:inspection module:metrics labels Feb 4, 2024

DOC Adds PR number

537bdb2

thomasjpfan mentioned this pull request Feb 4, 2024

[API] A public API for creating and using multiple scorers in the sklearn-ecosystem #28299

Closed

TST More interesting dict test

3a5946d

eddiebergman reviewed Feb 5, 2024

View reviewed changes

DOC Fixes docstring

7a71a83

adrinjalali reviewed Feb 6, 2024

View reviewed changes

thomasjpfan and others added 2 commits February 7, 2024 10:48

Merge remote-tracking branch 'upstream/main' into get_scorer_multimetric

61b99a6

Merge branch 'main' into get_scorer_multimetric

3a981a9

glemaitre self-requested a review February 8, 2024 18:32

thomasjpfan added 2 commits February 17, 2024 11:40

Merge remote-tracking branch 'upstream/main' into get_scorer_multimetric

94834f6

MNT Adds repr for scorer

867cc10

adrinjalali approved these changes Feb 19, 2024

View reviewed changes

glemaitre approved these changes Feb 19, 2024

View reviewed changes

glemaitre merged commit b3c3f05 into scikit-learn:main Feb 19, 2024

glemaitre mentioned this pull request May 3, 2024

Allow for multiple scoring metrics in RFECV #28937

Open

StefanieSenger mentioned this pull request May 10, 2024

ENH check_scoring() has raise_exc for multimetric scoring #28992

Merged

bpkroth mentioned this pull request Aug 6, 2024

Update the matrix of supported/tested combinations of sklearn and python used in CI dabl/dabl#343

Merged

	def _get_scorers(self, convert_multimetric):
	"""Get the scorer(s) to be used.

	This is used in ``fit`` and ``get_metadata_routing``.

	Parameters
	----------
	convert_multimetric : bool
	Whether to convert a dict of scorers to a _MultimetricScorer. This
	is used in ``get_metadata_routing`` to include the routing info for
	multiple scorers.

	Returns
	-------
	scorers, refit_metric
	"""
	refit_metric = "score"

	if callable(self.scoring):
	scorers = self.scoring
	elif self.scoring is None or isinstance(self.scoring, str):
	scorers = check_scoring(self.estimator, self.scoring)
	else:
	scorers = _check_multimetric_scoring(self.estimator, self.scoring)
	self._check_refit_for_multimetric(scorers)
	refit_metric = self.refit
	if convert_multimetric and isinstance(scorers, dict):
	scorers = _MultimetricScorer(
	scorers=scorers, raise_exc=(self.error_score == "raise")
	)

	return scorers, refit_metric

	names and the values are the metric scores;
	names and the values are the metric scorers;

Uh oh!

Conversation

thomasjpfan commented Feb 4, 2024

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

github-actions bot commented Feb 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

eddiebergman left a comment

Choose a reason for hiding this comment

Uh oh!

eddiebergman Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

thomasjpfan Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

eddiebergman Feb 5, 2024

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Feb 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

thomasjpfan commented Feb 7, 2024

Uh oh!

glemaitre commented Feb 8, 2024

Uh oh!

eddiebergman commented Feb 8, 2024

Uh oh!

glemaitre commented Feb 8, 2024

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

glemaitre left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

github-actions bot commented Feb 4, 2024 •

edited

Loading

thomasjpfan commented Feb 5, 2024 •

edited

Loading