ENH: externalize MapieClassifier methods into several non-conformity scores #476

thibaultcordier · 2024-07-01T14:52:32Z

Description

Outsource the MapieClassifier methods (LAC, APS, RAPS, TOPK) by extending the non-conformity score class to the classification task.

Fixes #467

Type of change

New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Test that existing code continues to work
Test that the proposed solution can enable new non-conformity scores

Checklist

I have read the contributing guidelines
I have updated the HISTORY.rst and AUTHORS.rst files
Linting passes successfully : make lint
Typing passes successfully : make type-check
Unit tests pass successfully : make tests
Coverage is 100% : make coverage
Documentation builds successfully : make doc

…ores + generic adaptation

…or classification score

… classification

vincentblot28 · 2024-07-04T09:02:57Z

What a tour de force @thibaultcordier !! Great improvement to the classification API !
General comment about the example: as in the future we prefer to use directly the conformity scores (and the argument "method" might be removed) I think examples should follow the new way of using MapieClassifier

mapie/classification.py

mapie/conformity_scores/sets/aps.py

vincentblot28 · 2024-07-04T09:35:49Z

mapie/conformity_scores/sets/aps.py

+from sklearn.dummy import check_random_state
+
+from mapie.conformity_scores.sets.naive import Naive
+from mapie.conformity_scores.sets.utils import get_true_label_cumsum_proba


can we add get_true_label_cumsum_proba to the class method ? because it's only used for APS and RAPS. So if it's class method of APS, as RAPS inherits from APS, it will also be a method of RAPS

mapie/conformity_scores/sets/naive.py

vincentblot28 · 2024-07-04T09:41:19Z

mapie/conformity_scores/sets/naive.py

+        )
+
+        # get the V parameter from Romano+(2020) or Angelopoulos+(2020)
+        vs = self._compute_vs_parameter(


there is no vs need in the naive method

vs is needed when include_last_label == "randomized", do we have a condition that prohibits method='naive' and include_last_label='randomized'?

yes we should, acutually naive method should always have include_last_label=True

This condition is not currently verified, is that correct? Should this therefore be the subject of another PR?

I think we can add it in this PR, it will make the code easier to read

@LacombeLouis do you agree ?

To what extent is it linked to this issue #456 ?

it's not linked

vincentblot28 · 2024-07-04T12:14:47Z

mapie/conformity_scores/sets/naive.py

+        conformity_scores: NDArray,
+        alpha_np: NDArray,
+        estimator: EnsembleClassifier,
+        agg_scores: Optional[str] = "mean",


Can you includ agg_scores and include_last_label to the kwargs as none of them is used for naive conformity score ?

mapie/conformity_scores/sets/raps.py

LacombeLouis

Hey @thibaultcordier,
Thank you for this PR. Extremely well-thought-out and very detailed. Some small questions here and there. Looking forward to have this new PR included!

mapie/_machine_precision.py

mapie/conformity_scores/bounds/residuals.py

LacombeLouis · 2024-07-02T15:47:07Z

mapie/conformity_scores/bounds/absolute.py

+    The conformity score is symmetrical.
+
+    This is appropriate when the confidence interval is symmetrical and
+    its range is approximatively the same over the range of predicted values.


Could we point to a source for this method?

This is not the point of this PR.

Could we create an issue then?

mapie/conformity_scores/bounds/gamma.py

LacombeLouis · 2024-07-05T07:19:45Z

mapie/conformity_scores/sets/aps.py

+            y_pred, y_enc.reshape(-1, 1), axis=1
+        )
+        random_state = check_random_state(self.random_state)
+        random_state = cast(np.random.RandomState, random_state)


Does the check_random_state not already do this?

It also manages the default parameters (if None or if integer)

LacombeLouis · 2024-07-05T07:20:10Z

mapie/conformity_scores/sets/aps.py

+        random_state = check_random_state(self.random_state)
+        random_state = cast(np.random.RandomState, random_state)
+        u = random_state.uniform(size=len(y_pred)).reshape(-1, 1)
+        conformity_scores -= u * y_proba_true


I would rather we use numpy to make multiplications.

It was like that before, I didn't touch it during the refactoring

LacombeLouis · 2024-07-05T07:21:11Z

mapie/conformity_scores/sets/aps.py

+        if estimator.cv == "prefit" or agg_scores in ["mean"]:
+            quantiles_ = compute_quantiles(conformity_scores, alpha_np)
+        else:
+            quantiles_ = (n + 1) * (1 - alpha_np)


Why don't we use compute quantiles?

It was like that before, I didn't touch it during the refactoring

LacombeLouis · 2024-07-05T07:26:03Z

mapie/tests/test_classification.py

@@ -1606,28 +1612,16 @@ def test_method_error_in_fit(monkeypatch: Any, method: str) -> None:
        mapie_clf.fit(X_toy, y_toy)


-@pytest.mark.parametrize("method", WRONG_METHODS)


Why do we no longer perform this test?

This test tested, in the case where the user changes the method value between fit and predict, that an error was reported.

But now, we manage non-conformity scores. Due to the logic of the refacto, this test in the code no longer makes sense because we do not modify the non-confnormity score as we want.

We do not do this test for MapieRegressor

We do not do this test if the estimator has changed between the fit and the predict

LacombeLouis · 2024-07-05T07:41:31Z

mapie/tests/test_conformity_scores_sets.py

+
+
+cs_list = [None, LAC(), APS(), TopK()]
+method_list = [None, 'naive', 'aps', 'raps', 'lac', 'top_k']


Should we add wrong methods here? (and a test to go with it)

UPD: factorize MapieClassifier methods into several non-conformity sc…

d3bcba5

…ores + generic adaptation

thibaultcordier added enhancement New feature or request developers Proposed by developers. labels Jul 1, 2024

thibaultcordier requested review from vincentblot28 and LacombeLouis July 1, 2024 14:52

thibaultcordier self-assigned this Jul 1, 2024

thibaultcordier linked an issue Jul 1, 2024 that may be closed by this pull request

Extending the NCS class to the classification task #467

Open

thibaultcordier changed the title ~~UPD: externalize MapieClassifier methods into several non-conformity scores~~ ENH: externalize MapieClassifier methods into several non-conformity scores Jul 1, 2024

Thibault Cordier added 10 commits July 1, 2024 16:58

FIX: path access in test doctring

0eb7203

FIX: adapt exemple code with new signatures

fc0f46a

UPD: check tests for additional parameters in residual normalized score

f064cc9

UPD: typing and doctring in score classes

a9c03fd

UPD: use y_enc as additional parameters to conserve label encoding

7f791dd

UPD: change greater equal to less equal

1b99529

UPD: docstring to explain parameters of get_sets

e588a3e

UPD: remove useless methods et attributes (estimation distribution) f…

8832a2e

…or classification score

UPD: decompose APS into Naive, APS and RAPS + new abtract methods for…

8fa2474

… classification

UPD: improve docstring of score classes

c79d6e5

thibaultcordier marked this pull request as ready for review July 3, 2024 16:20

UPD: refacto changes in history file

59586a9

vincentblot28 reviewed Jul 4, 2024

View reviewed changes

mapie/classification.py Outdated Show resolved Hide resolved

vincentblot28 reviewed Jul 4, 2024

View reviewed changes

mapie/conformity_scores/sets/aps.py Show resolved Hide resolved

vincentblot28 reviewed Jul 4, 2024

View reviewed changes

mapie/conformity_scores/sets/naive.py Outdated Show resolved Hide resolved

vincentblot28 reviewed Jul 4, 2024

View reviewed changes

mapie/conformity_scores/sets/naive.py Outdated Show resolved Hide resolved

vincentblot28 reviewed Jul 4, 2024

View reviewed changes

mapie/conformity_scores/sets/raps.py Outdated Show resolved Hide resolved

UPD: add missing attributes + keep quantiles_ attribute

7d053b7

Thibault Cordier added 3 commits July 4, 2024 16:22

UPD: remove obsolete 'method' attribute and methods in conformity score

15b31ff

UPD: reduce doctring

ee89b53

UPD: change method name

d9e4989

LacombeLouis reviewed Jul 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: externalize MapieClassifier methods into several non-conformity scores #476

ENH: externalize MapieClassifier methods into several non-conformity scores #476

thibaultcordier commented Jul 1, 2024 •

edited

Loading

vincentblot28 commented Jul 4, 2024

vincentblot28 Jul 4, 2024

vincentblot28 Jul 4, 2024

thibaultcordier Jul 4, 2024

vincentblot28 Jul 4, 2024

thibaultcordier Jul 4, 2024

vincentblot28 Jul 4, 2024

vincentblot28 Jul 4, 2024

LacombeLouis Jul 4, 2024

vincentblot28 Jul 5, 2024

vincentblot28 Jul 4, 2024

LacombeLouis left a comment

LacombeLouis Jul 2, 2024

thibaultcordier Jul 5, 2024

LacombeLouis Jul 5, 2024

LacombeLouis Jul 5, 2024

thibaultcordier Jul 5, 2024

LacombeLouis Jul 5, 2024

thibaultcordier Jul 5, 2024

LacombeLouis Jul 5, 2024

thibaultcordier Jul 5, 2024

LacombeLouis Jul 5, 2024

thibaultcordier Jul 5, 2024

LacombeLouis Jul 5, 2024

		@@ -1606,28 +1612,16 @@ def test_method_error_in_fit(monkeypatch: Any, method: str) -> None:
		mapie_clf.fit(X_toy, y_toy)


		@pytest.mark.parametrize("method", WRONG_METHODS)



		cs_list = [None, LAC(), APS(), TopK()]
		method_list = [None, 'naive', 'aps', 'raps', 'lac', 'top_k']

ENH: externalize MapieClassifier methods into several non-conformity scores #476

Are you sure you want to change the base?

ENH: externalize MapieClassifier methods into several non-conformity scores #476

Conversation

thibaultcordier commented Jul 1, 2024 • edited Loading

Description

Type of change

How Has This Been Tested?

Checklist

vincentblot28 commented Jul 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LacombeLouis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thibaultcordier commented Jul 1, 2024 •

edited

Loading