Fix calibration self-inclusion leak in NeighborhoodLabel (NCP) - #1217
Open
lehendo wants to merge 2 commits into
Open
Fix calibration self-inclusion leak in NeighborhoodLabel (NCP)#1217lehendo wants to merge 2 commits into
lehendo wants to merge 2 commits into
Conversation
calibrate() queried sklearn's NearestNeighbors.kneighbors() with an explicit X argument equal to the fitted calibration set, which (unlike the implicit no-argument form) returns each point as its own nearest neighbor at distance 0. This let every calibration point's own score dominate its own threshold during the alpha_tilde search -- something a genuine test point, never part of the calibration set, can't benefit from -- biasing the search toward an overly permissive threshold. Verified against the cited paper (Ghosh et al., AAAI 2023, arXiv:2303.10694): the paper's own formulas (Eq. 2, 4-5) are silent on whether self should be excluded, so this isn't a literal equation mismatch, but leaving it in violates the exchangeability premise conformal prediction depends on. Confirmed via Monte Carlo simulation: 0.82-0.83 actual coverage against a 0.90 target with the leak, 0.89-0.90 with leave-one-out; confirmed 0/50 self-matches remain after the fix in the real class. Also completed a fuller audit of NCP against the paper's full Algorithm 2 pseudocode: - Core weighted-quantile formula and alpha_tilde search structure confirmed correct. - Marginal-only coverage (no class-conditional alpha) confirmed to correctly match the paper's own scope, not a gap. - k_neighbors/lambda_L defaults confirmed within the paper's tested range. - Documented two deliberate deviations from a literal reading of the paper: the leave-one-out fix itself, and the empty-prediction-set fallback (coverage-safe, since it can only add to a set). - Documented that, like standard split-conformal, this method assumes exchangeability and does not correct for covariate shift. Fixed an existing test that had silently reimplemented the same self-inclusive (buggy) logic inside itself, so it validated the bug's own leakage rather than catching it. Added a direct regression test for the self-inclusion leak. 11 tests pass.
CI failed intermittently on test_calibration_empirical_coverage_at_least_1_minus_alpha
(0.6667 not >= 0.8). Root-caused two distinct issues, not just one:
1. The shared 6-sample fixture makes only {0, 1/6, ..., 1} achievable as
coverage values, so the 0.8 target sits squarely between 4/6=0.667 and
5/6=0.833 -- a single point flipping either way (driven by the model's
unseeded random init; setUp() never calls torch.manual_seed) flips the
assertion. This alone explains the observed intermittent CI failure.
2. More fundamentally: the shared fixture's model is never trained, so its
conformity scores are pure noise from a random init, uncorrelated with
embedding-space locality. NCP's per-point threshold comes from each
point's k-nearest neighbors only (itself excluded) rather than a global
quantile over all N points, so -- unlike plain split conformal -- it has
no automatic 'some alpha_tilde>=0 must reach 1-alpha coverage' guarantee
independent of whether the scores actually correlate with locality.
Confirmed directly: with the untrained model, alpha_tilde_ converges to
its floor of 0.0 (the search's most permissive setting) and still only
covers 23/30 -- consistent with roughly 1/(k+1) of points having a lower
score than all k neighbors purely by chance under pure noise.
Fixed by giving this specific test its own seeded, larger (N=30), briefly
*trained* dataset/model rather than reusing the shared untrained 6-sample
fixture (left untouched for the other tests in this file that depend on
it). Verified reliably passing across 6 independent seeds (1, 2, 3, 7, 42,
100), not just the one used in the final version.
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
1 participant
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.
calibrate() queried sklearn's NearestNeighbors.kneighbors() with an explicit X argument equal to the fitted calibration set, which (unlike the implicit no-argument form) returns each point as its own nearest neighbor at distance 0