feat!: split calibration into naive and multi-head-aware classes - #121
Merged
RobbinBouwmeester merged 1 commit intoSep 8, 2026
Merged
Conversation
Split deeplc/calibration.py into a package: calibration/simple.py keeps the naive, single-series classes unchanged; calibration/multihead.py adds the MultiHeadCalibration ABC, MultiHeadPiecewiseLinearCalibration, MultiHeadSplineCalibration, and the relocated MultiHeadRidgeCalibration. calibrate()/predict_and_calibrate() always hand over the full (n, n_heads) prediction matrix and default to MultiHeadRidgeCalibration for every model, dropping the uses_all_heads/selected_model_head branching in core.py. Add upgrade_calibration() to keep accepting a naive, unfitted Calibration (e.g. SplineTransformerCalibration()) by wrapping it in its MultiHead* counterpart, so existing callers of calibrate()/ predict_and_calibrate() aren't broken by the split. Fix IdentityCalibration(), which could not be instantiated (missing __init__ override). BREAKING CHANGE: predict_and_calibrate() rejects an already-fitted naive Calibration; fit a MultiHead*Calibration instead. Calibration .selected_model_head/.uses_all_heads are removed.
RobbinBouwmeester
approved these changes
Sep 8, 2026
RobbinBouwmeester
added a commit
that referenced
this pull request
Sep 8, 2026
main split calibration.py into a package (#121): simple.py keeps the naive classes, multihead.py holds the MultiHeadCalibration ABC and the relocated MultiHeadRidgeCalibration, every caller now hands over the full (n, n_heads) matrix, and the uses_all_heads branching is gone. This branch had added a disagreement hook to the old single module, so the two collided in calibration/simple.py. Resolved by following the new layout rather than the diff: - simple.py takes main's version. The disagreement hook moves to the MultiHeadCalibration ABC, where it belongs: only a calibration that combines several estimates of one retention time has a spread to report, and a naive single-series class never did. - MultiHeadRidgeCalibration.disagreement lands in multihead.py, sharing the new _calibrated_columns helper with transform. - report.py loses both uses_all_heads branches. _crossfit_residuals and prediction_report now fit and transform on the full matrix for every calibration, and a naive one passed by a caller goes through upgrade_calibration first, so the report follows the same contract as calibrate() and predict_and_calibrate(). - selected_heads for the membership columns comes from the calibration's own _head_idx, falling back to selected_model_head for a single-head fit. Both paths verified on PXD081924: MultiHeadRidgeCalibration gives MAE 0.272 min, coverage 0.925 and 1,051 distinct interval widths, matching this branch before the merge; a naive SplineTransformerCalibration, auto-upgraded, gives MAE 0.351, coverage 0.932 and the five retention-time-binned widths expected when a single head reports no disagreement. 187 tests pass, ruff clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Sep 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split deeplc/calibration.py into a package: calibration/simple.py keeps the naive, single-series classes unchanged; calibration/multihead.py adds the MultiHeadCalibration ABC, MultiHeadPiecewiseLinearCalibration, MultiHeadSplineCalibration, and the relocated MultiHeadRidgeCalibration. calibrate()/predict_and_calibrate() always hand over the full(n, n_heads) prediction matrix and default to MultiHeadRidgeCalibration for every model, dropping the uses_all_heads/selected_model_head branching in core.py.
Add upgrade_calibration() to keep accepting a naive, unfitted Calibration (e.g. SplineTransformerCalibration()) by wrapping it in its MultiHead* counterpart, so existing callers of calibrate()/predict_and_calibrate() aren't broken by the split.
Fix IdentityCalibration(), which could not be instantiated (missing init override).
BREAKING CHANGE: predict_and_calibrate() rejects an already-fitted naive Calibration; fit a MultiHead*Calibration instead. Calibration .selected_model_head/.uses_all_heads are removed.