P3-07: the calibration stack — calib-only temperature scaling + Saerens/Elkan prior-shift - #100
Merged
Merged
Conversation
…ng + Saerens/Elkan prior-shift
Inputs: calib/temperature.py::{fit_temperature, apply_temperature, STACK_ORDER} (P2-13);
the P3-02 `calib` column contract; stage2/model.py "tbox_logit" (B,) raw 1-logit head.
Outputs: src/tbox_finder/calib/recalibrate.py::{temperature_scale, prior_shift,
calibrated_posterior, assign_rung, rung_labels, prior_from_odds_ratio,
deployment_prior_in_prd_band}; tests/unit/test_recalibrate.py; dev-log stanza.
ADR-0005 D11 pins the stack order (train -> temperature-scale -> prior-shift) and, more
consequentially, which posterior is graded: GATE-2's in-distribution ECE is measured on the
NAMED posterior, temperature-scaled and PRE prior-shift. calibrated_posterior therefore
returns both, with gated_posterior_key naming the former inside the payload.
One temperature fit, not two. Stage 2's head is a single logit, and stacking it as [0, z]
makes P2-13's multi-class fit identically the binary one (softmax([0, bz])[1] = sigmoid(bz)) —
the same reduction stage2/losses.py already uses for the binary term. The adapter changes
inputs, never arithmetic, so P2-13's degenerate-limit refusals carry through. Delegation makes
"A agrees with B" a tautology, so the primary test is a hand-derived closed form:
z=[+a,+a,-a], y=[1,0,0] => 3*sigmoid(b*a)=2 => b* = ln2/a exactly, plus a second root at a
second scale, a ternary search in the test (minimises the value, not the derivative's root),
and a manufactured-miscalibration recovery of a known T.
"T fit only on calib, never test" is structural: temperature_scale takes the whole table plus
a per-row rung and selects the calib rows itself. Around it — an emptiness raise (an
all-False calib column filters clean and fits on nothing, which reads exactly like a working
filter), an unknown-rung-token raise (a vanished row shrinks the set while the counts look
fine), a single-class raise, and assign_rung re-deriving the P3-02 invariant with
masking.is_missing (bool(nan) is True, and pandas 3 delivers nulls as NaN). Fixtures are
asymmetric 7/4/3/5 and assert identity both ways round.
No deployment prior is pinned and none is invented: ~10^3-10^4:1 is PRD prose only, so both
priors are required kwargs with no defaults, a half-specified shift raises, and the PRD band
is reporting-only (shown to discriminate — D7's own 100:1 falls outside it).
No T is fitted here: nothing in the repo reads a Stage-2 checkpoint back, so real calib-split
logits do not exist yet. The score producer belongs to P3-08/P3-10.
Path drift recorded in imp.md: calib/recalibrate.py, not calibration/recalibrate.py — the
calib package already exists and the block's sibling calibration/ paths drifted at P0-31 too.
Validation: 108/108 unit (test_recalibrate), test_temperature 81 unchanged; ruff 0.15.15 +
black 25.11.0 clean; 15 source sabotages, each RED against its NAMED test, each restored
byte-identically. No ADR amendment, no §7 sign-off: D11 is implemented verbatim and the one
value that would need a pin is the one the module refuses to supply.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 25 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR adds the Stage-2 recalibration stack. It derives calibration rungs, fits temperature on ChangesStage-2 recalibration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant recalibrate
participant temperature_scale
participant prior_shift
participant PosteriorOutput
Caller->>recalibrate: provide logits, labels, rungs, and optional priors
recalibrate->>temperature_scale: fit temperature on calib rows
temperature_scale-->>recalibrate: return fitted temperature and metadata
recalibrate->>prior_shift: apply prior correction when both priors exist
prior_shift-->>recalibrate: return shifted posterior
recalibrate->>PosteriorOutput: return gated and shifted posterior outputs
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/tbox_finder/calib/recalibrate.py (1)
104-106: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
STEP,GENERATED_BY, andADRare defined but never read.No function in this module reads these three constants, and they are absent from
__all__and from thecalibrated_posteriorpayload. The payload already carriesstack_orderandgated_posterior_keyfor provenance, so add these fields to it, or drop the constants. The test module docstring names this exact pattern ([[pinned-constant-that-nothing-reads]]).♻️ Option: carry the provenance into the payload
payload: dict[str, Any] = { + "step": STEP, + "generated_by": GENERATED_BY, + "adr": ADR, "stack_order": list(STACK_ORDER),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/tbox_finder/calib/recalibrate.py` around lines 104 - 106, Update the calibrated_posterior payload construction to include the existing STEP, GENERATED_BY, and ADR constants as provenance fields, alongside stack_order and gated_posterior_key; ensure the constants are actually read by that payload rather than removing them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/tbox_finder/calib/recalibrate.py`:
- Around line 371-376: Validate label integrality on the original y values
before the astype(np.int64, copy=False) conversion in the recalibration flow.
Reject fractional and non-finite labels with the existing validation error path,
then perform the int64 cast and retain the binary 0/1 range check for integral
values.
---
Nitpick comments:
In `@src/tbox_finder/calib/recalibrate.py`:
- Around line 104-106: Update the calibrated_posterior payload construction to
include the existing STEP, GENERATED_BY, and ADR constants as provenance fields,
alongside stack_order and gated_posterior_key; ensure the constants are actually
read by that payload rather than removing them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 761adeef-b560-4ab1-b3ab-32eca4da6fad
⛔ Files ignored due to path filters (1)
analyses/phase3_log.qmdis excluded by!**/*.qmd
📒 Files selected for processing (3)
src/tbox_finder/calib/__init__.pysrc/tbox_finder/calib/recalibrate.pytests/unit/test_recalibrate.py
…oolean parse instead of forking it CodeRabbit r1 on 88879fd (1 actionable + 1 nitpick) plus a 44-agent adversarial pass (20 findings, 10 surviving two independent refuters), deduplicated to six fixes. Both sources independently found the same defect at the int64 cast. 1. The "labels must be binary 0/1" guard was VACUOUS: it ran after astype(np.int64), which truncates toward zero. Demonstrated — temperature_scale(z=[1,1,-1], y=[1.0,0.4,0.6]) SUCCEEDS and returns T bit-identical to the fit for [1,0,0]; a target written as 0.6 was silently scored a negative. A NaN label cast to the int64 sentinel and surfaced as "range [-9223372036854775808, 1]". The domain check now happens in the original dtype: bool/int pass, floats must be finite and integral, any other dtype is refused by name. 2. bool(calib) closed the NaN hole and left the string hole open. The guard exists BECAUSE pandas 3 delivers a null as NaN and bool(nan) is True; one step out, bool("False") and bool("0") are also True, so a string-typed calib column would promote its explicit negatives into the set T is fitted on with every count agreeing. Reproduced: rung_labels(calib=["True","False",...]) returned ["calib"]*6. The fix was already in the repo. stage2/train.py::_bool_or_none is exactly this parse and is documented as also reading `calib`, but cannot be imported here (that module is torch-adjacent; this one is deliberately numpy-only). So it is PROMOTED DOWN to masking.py, beside the is_missing/row_text it completes, with NULL_TOKENS moving with it. stage2.heads.NULL_TOKENS is now a re-export and stage2.train._bool_or_none a delegation: one vocabulary, one parse, three call sites. A fourth copy would have been the cheaper diff and the wrong one. 3. The single-class guard is a disjunct (n_pos == 0 or n_pos == n_fitted) and only the all-positive half was tested; with the other half deleted, an all-negative calib carve returns a cleanly converged, fully certified T. Both halves now parametrized. 4. test_shape_and_domain_violations_raise used a bare pytest.raises(ValueError) — both of this module's guards could be deleted and it stayed green, because the delegated fitter raises its own error further in. Now matches this module's own text. 5. The empty-calib refusal asserted only its first clause, leaving the census and the GRADED_RUNGS count (the only place that constant is read) unchecked. 6. The payload's source_prior/target_prior were write-only: swapping the two assignments left the suite green while recording the deployment prior as the calib prevalence. Also carries step/generated_by/adr now (CodeRabbit nitpick: three constants nothing read). Citation: "Elkan 2001, Theorem 1" -> Elkan 2001 with no theorem number. A reviewer put the base-rate result at Theorem 2; the numbering could not be verified from here, and a pointer to the wrong theorem is worse than one to the paper. Validation: test_recalibrate 138 pass (was 108); test_temperature 81; masking + heads + stage2_train_smoke 411 pass together (the promotion is behaviour-preserving on both sides). Full unit+golden+ml: the worktree's 14 failures are a strict SUBSET of the 31 untouched `main` produces in this local runner (py3.11/numpy 2.4.2) — zero regressions. ruff 0.15.15 + black 25.11.0 clean. 24 source sabotages (up from 15), each RED against its NAMED test, each restored byte-identically.
…still refuse the neighbours The GitHub CodeRabbit check read `pass` on a "Review rate limited" body and no review had run on a721d06 (CLAUDE.md §5.1: a rate-limit notice is an absence, not a pass), so this round came from rung 1 of the ladder — the CodeRabbit CLI, scoped with --committed --base-commit 88879fd. One minor finding, and it is correct. masking.bool_or_none stringifies before matching, so a float flag becomes "1.0"/"0.0", matches no spelling, and raises. That reads like fail-closed behaviour on a schema fault until the shape is checked: pd.Series([True, None, False]).astype(float) and a `boolean`-dtype cast to float64 BOTH produce 1.0 / nan / 0.0. The NaN was already the missing case; the 1.0 and 0.0 beside it are a well-formed nullable-boolean column, and refusing them aborts a legitimate run. The parse now accepts a finite Real equal to exactly 0 or 1 before the text path. 0.5, 1.5, -0.5, 2 and -1 still raise — those are schema faults and the widening does not touch them. This is inherited behaviour, not new: stage2/train.py::_bool_or_none has had it since P3-06, and it only became visible because r1 moved the parse to a shared home where a reviewer read it. Fixing it once fixes it for the fold readers too — the argument for promoting rather than forking, paying off one round later. Validation: test_recalibrate 146 pass; masking + heads + stage2_dataset + stage2_train_smoke + temperature + no_leakage 502 pass beside it. Full unit+golden+ml: 14 failures, still a strict subset of main's 31 in this local runner (comm -13 over the two ^FAILED sets is empty). ruff + black clean. Two more sabotages, both bitten and in both directions: removing the acceptance reddens the float test, and widening it to any finite real (0.5 -> True) reddens the refusal test.
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.
Implements imp.md step P3-07 — the ADR-0005 D11 recalibration stack for the Stage-2 binary head. LOCAL only: no GPU-h, no SLURM, no DVC artifact.
What lands
src/tbox_finder/calib/recalibrate.py—temperature_scale,prior_shift,calibrated_posterior, plusassign_rung/rung_labels/prior_from_odds_ratio/deployment_prior_in_prd_band. numpy-only (no scipy, no sklearn, no torch), so the whole unit tier runs in CI's stack as-is.D11 pins the stack order and which posterior is graded: GATE-2's in-distribution ECE is measured on the named posterior — temperature-scaled, before the prior shift.
calibrated_posteriortherefore returns both, withgated_posterior_keynaming the gated one inside the payload so a downstream report cannot quietly grade the wrong object.One temperature fit, not two
Stage 2's head is a single logit. Stacking it as
[0, z]makes P2-13's multi-classcalib/temperature.py::fit_temperatureidentically the binary fit, sincesoftmax([0, βz])[1] = σ(βz)— the same reductionstage2/losses.pyalready uses to route the binary term through the audited focal-CE kernel. The adapter changes inputs, never arithmetic, so P2-13's two degenerate-limit refusals (perfect separation, worse-than-uniform) carry through unchanged.Delegating makes "A agrees with B" a tautology, so the primary test is neither: for
z=[+a,+a,−a],y=[1,0,0]the stationarity condition collapses to3σ(βa)=2, givingβ* = ln2/aexactly. Backed by a second root at a second scale (4σ=3), a ternary search in the test that minimises the value rather than rooting the derivative, and a manufactured-miscalibration draw recoveringT ≈ 2.5from 20k rows. The delegation check is present and labelled secondary."T fit only on
calib, never test" — structural, not assertedtemperature_scaletakes the whole table plus a per-row rung label and selects thecalibrows itself, so no argument admits a graded row to the fit. Around that:Falsecalib column filters clean and fits on nothing, which reads exactly like a working filter;assign_rungre-derives the P3-02 invariant (a calib row lives in thetrainrung) and tests missingness withmasking.is_missing, becausebool(float("nan")) is Trueand pandas 3 delivers nulls as NaN — the naive guard would promote a row with no calibration flag into the fit.Fixtures are asymmetric (7 calib / 4 train / 3 val / 5 test) with both the calib and test arms independently fittable, so the inversion test runs both ways: swapping the senses is asserted to yield exactly the test arm's own
T. A balanced, count-only fixture would refuse the right number of the wrong rows and pass.Two numbers this step declines to produce
~10³–10⁴:1is prose only (PRD.md:188, ADR-0005:80); nothing in the repo encodes a value (priors.pyis the unrelated union novelty prior). Pinning a scalar would be a new blinded-frozen default needing ADR-0005 re-sign-off, so both priors are required kwargs with no defaults, a half-specified shift raises, andDEPLOYMENT_PRIOR_RANGEis reporting-only — with the predicate shown to discriminate, since ADR-0005 D7's own100:1benchmark prevalence must fall outside it.Tis fitted. Nothing in the repo reads a Stage-2 checkpoint back (stage2_heads.pt/lora_adapter/have write sites and zero readers;train.py::evaluatediscards per-row logits), so realcalib-split logits do not exist yet. This ships and certifies the machinery; the score producer, and the first realT, belong to P3-08/P3-10. ATinvented to fill the gap would look exactly like a measured one.Path drift (recorded in imp.md)
imp.md named
src/tbox_finder/calibration/recalibrate.py; it lands atsrc/tbox_finder/calib/recalibrate.py— thecalib/package already exists, and two near-homonymous calibration namespaces is a permanent import confusion. That block's siblingcalibration/paths already drifted on their own (P3-09'scalibration/ece.py::binned_eceshipped asmetrics.py::binned_eceat P0-31). Same shape as the recorded P3-02 correction.Validation
tests/unit/test_recalibrate.py108 pass;test_temperature81 pass, unchanged.is_missingweakened tois None; P3-02 invariant dropped; shift direction inverted;+flipped to−; gated key repointed at the shifted posterior; a deployment prior given a default; half-specified guard removed; PRD band widened to swallow D7's100:1; branch-wise sigmoid replaced by the overflowing one; single-class guard removed; priors of exactly 0/1 admitted;prior_from_odds_ratioinverted.No ADR amendment and no §7 sign-off taken: D11 pins the stack order and the named posterior, both implemented verbatim, and the one value that would have needed a pin is the one the module refuses to supply.
Summary by CodeRabbit
New Features
Bug Fixes