-
Couldn't load subscription status.
- Fork 127
FEAT: Implement VennAbers calibration method for both binary and mult… #777
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.
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
base: master
Are you sure you want to change the base?
Conversation
…i-class classification (based on ip200/venn-abers github repository implementation)
|
The result of I will review the server logs and get back to you. |
|
Thanks for the PR @dimoibiehg ! |
|
Adding a bit of context here: #736 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Implements Venn–Abers calibration for binary and multi-class classification, adding a new calibrator class, core VA algorithms, and extensive tests.
- Adds VennAbersCalibrator to mapie.calibration with prefit, inductive (IVAP), and cross (CVAP) modes.
- Introduces core Venn–Abers implementations (binary, CV, multiclass) in mapie/_venn_abers.py.
- Adds comprehensive unit tests and updates HISTORY and AUTHORS.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| mapie/calibration.py | Adds VennAbersCalibrator with fit/predict_proba/predict logic and pipeline handling. |
| mapie/_venn_abers.py | New internal module implementing VA core algorithms (binary, CV, multiclass) and helper utilities. |
| mapie/tests/test_venn_abers_calibration.py | Extensive tests covering modes, estimators, pipelines, weights, precision, and error cases. |
| HISTORY.rst | Notes the introduction of Venn–Abers calibrator. |
| AUTHORS.rst | Adds contributor. |
| mapie/utils.py | Minor whitespace cleanup. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
mapie/calibration.py
Outdated
| # Validate inputs | ||
| X, y = indexable(X, y) | ||
| y = _check_y(y) | ||
| sample_weight, X, y = _check_null_weight(sample_weight, X, y) | ||
| # Handle categorical features |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indexable is used but never imported in this module, which will lead to a NameError at runtime. Import it with from sklearn.utils import indexable near the other imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not correct, indexable has been imported from sklearn.utils.validation
| if isinstance(last_estimator, Pipeline): | ||
| # Separate transformers and final estimator | ||
| transformers = self.estimator[:-1] # all steps except last | ||
| last_estimator = self.estimator[-1] # usually a classifier | ||
|
|
||
| X_processed = transformers.fit_transform(X, y) | ||
| self.transformers_ = transformers |
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fitting the pipeline transformers on the full X before any inductive/CV split causes data leakage; in prefit mode it also decouples the transformer state from the estimator that was trained on a different transformer fit. Pass the full Pipeline to the underlying calibrator so each fold/split fits transformations on training data, and avoid refitting transformers in prefit mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The estimator is fitted in the prefit mode; however, the transformers might not!
mapie/calibration.py
Outdated
| if (self.transformers_ is not None): | ||
| X_processed = self.transformers_.transform(X) | ||
| else: | ||
| X_processed = X | ||
| # Prefit mode: use fitted estimator to get probabilities, then calibrate | ||
| if cv == "prefit": | ||
| if self.single_estimator_ is None: | ||
| raise RuntimeError( | ||
| "single_estimator_ should not be None in prefit mode" | ||
| ) | ||
|
|
||
| p_test_pred = self.single_estimator_.predict_proba(X_processed) | ||
|
|
Copilot
AI
Oct 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In prefit mode with Pipeline estimators, using a transformer refit on calibration data (self.transformers_) to transform X for a classifier trained with a different transformer fit can produce feature mismatch. For prefit, call the fitted pipeline directly (no manual transform) or reuse the original fitted transformer from the pipeline instead of refitting it during fit.
| if (self.transformers_ is not None): | |
| X_processed = self.transformers_.transform(X) | |
| else: | |
| X_processed = X | |
| # Prefit mode: use fitted estimator to get probabilities, then calibrate | |
| if cv == "prefit": | |
| if self.single_estimator_ is None: | |
| raise RuntimeError( | |
| "single_estimator_ should not be None in prefit mode" | |
| ) | |
| p_test_pred = self.single_estimator_.predict_proba(X_processed) | |
| if cv == "prefit": | |
| if self.single_estimator_ is None: | |
| raise RuntimeError( | |
| "single_estimator_ should not be None in prefit mode" | |
| ) | |
| # If estimator is a pipeline, call it directly on X | |
| if isinstance(self.single_estimator_, Pipeline): | |
| p_test_pred = self.single_estimator_.predict_proba(X) | |
| else: | |
| if (self.transformers_ is not None): | |
| X_processed = self.transformers_.transform(X) | |
| else: | |
| X_processed = X | |
| p_test_pred = self.single_estimator_.predict_proba(X_processed) |
|
Thank you @dimoibiehg for getting back to us :) I just launched a Copilot review, but you can ignore it for now |
…tion not to be None
…n_classes_ variable
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #777 +/- ##
===========================================
Coverage 100.00% 100.00%
===========================================
Files 56 58 +2
Lines 6325 7894 +1569
Branches 360 434 +74
===========================================
+ Hits 6325 7894 +1569 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hello @dimoibiehg, I started reviewing your code. To make it clearer, could you explain what you did? From what I understood, you started from this repository, most code being in I noticed that you removed Finally, what is your progress on this PR? What remains to be done? Thanks a lot |
…i-class classification (based on ip200/venn-abers github repository implementation)
Description
Implement Vann-Abers calibration method for both binary and multi-class classification based on this repository.
Type of change
How Has This Been Tested?
A complete coverage test cases, implemented in mapie/tests/test_venn_abers_calibration.py
Checklist
make lintmake type-checkmake testsmake coveragemake docmake doctest