week_5: Module C (The Librarian) — C.3 confidence calibration (temperature scaling)#974
Open
PRAteek-singHWY wants to merge 3 commits into
Open
week_5: Module C (The Librarian) — C.3 confidence calibration (temperature scaling)#974PRAteek-singHWY wants to merge 3 commits into
PRAteek-singHWY wants to merge 3 commits into
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughAdds a temperature-scaling calibration module with error types and fitting/ECE utilities, unit tests, docstring updates, and live calibration gating in ChangesTemperature Calibration Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
PRAteek-singHWY
force-pushed
the
gsocmodule_C_week_5
branch
from
c03df58 to
fc68304
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/evaluate_librarian.py (1)
182-184: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winLive pipeline is built (and positives reranked) twice per run.
With
--use_live_embeddings,report_retrieval_recallandreport_calibrationeach call_build_live_pipeline(...)independently, so the DB connection, embedding model, and cross-encoder are loaded twice, and every positive row is retrieved+reranked in both reports. Since the cross-encoder load and per-pair inference are the expensive steps here, building the pipeline once inmainand passing(retriever, reranker)into both reports would roughly halve the live cost. This also matches the intent stated in_build_live_pipeline's docstring that the heavy hub + model load happens once.♻️ Sketch: build once in main, pass into reports
if args.use_live_embeddings: + retriever, reranker = _build_live_pipeline( + args.cache_file, + args.top_k_retrieval, + args.threshold, + args.top_k_rerank, + cfg.crossencoder_model, + ) - report_retrieval_recall( - rows, - args.cache_file, - args.top_k_retrieval, - args.threshold, - args.top_k_rerank, - cfg.crossencoder_model, - ) - calib_status = report_calibration( - rows, - args.cache_file, - args.top_k_retrieval, - args.threshold, - args.top_k_rerank, - cfg.crossencoder_model, - ) + report_retrieval_recall(rows, retriever, reranker, args.top_k_retrieval, args.top_k_rerank) + calib_status = report_calibration(rows, retriever, reranker)(Adjust the two report signatures to accept the prebuilt
retriever, reranker.)Also applies to: 238-251
🤖 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 `@scripts/evaluate_librarian.py` around lines 182 - 184, The live pipeline is being constructed twice because both report_retrieval_recall and report_calibration call _build_live_pipeline independently. Build the pipeline once in main by calling _build_live_pipeline(cache_file, top_k, threshold, top_n_rerank, crossencoder_model) a single time, then pass the resulting retriever and reranker into both report functions. Update the signatures of report_retrieval_recall and report_calibration to accept the prebuilt pipeline objects and remove their internal _build_live_pipeline calls.
🤖 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.
Nitpick comments:
In `@scripts/evaluate_librarian.py`:
- Around line 182-184: The live pipeline is being constructed twice because both
report_retrieval_recall and report_calibration call _build_live_pipeline
independently. Build the pipeline once in main by calling
_build_live_pipeline(cache_file, top_k, threshold, top_n_rerank,
crossencoder_model) a single time, then pass the resulting retriever and
reranker into both report functions. Update the signatures of
report_retrieval_recall and report_calibration to accept the prebuilt pipeline
objects and remove their internal _build_live_pipeline calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: e64443ca-ff6c-46c9-bff4-b4820fd64a97
📒 Files selected for processing (5)
application/tests/librarian/temperature_test.pyapplication/utils/librarian/__init__.pyapplication/utils/librarian/calibration/__init__.pyapplication/utils/librarian/calibration/temperature.pyscripts/evaluate_librarian.py
… once report_retrieval_recall and report_calibration each built the live pipeline (DB + embedding model + cross-encoder) independently, loading it twice and reranking every positive row twice per --use_live_embeddings run. Build it once in main and pass (retriever, reranker) into both reports, matching _build_live_pipeline's stated intent. Behavior-preserving: recall@20 285/292, rerank top-1 220/292, ECE 0.046 PASS unchanged.
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.
Hi @northdpole — Week 5 of Module C. This one turns the cross-encoder's raw score into an honest probability (temperature scaling), so Week 6 can threshold "auto-link vs. send-to-human" on a number that actually means what it says.
Overview
Week 3 built the search step (C.1) — a bi-encoder that cosine-ranks the whole CRE hub down to a top-20 shortlist. Week 4 built the rerank step (C.2) — a cross-encoder that reads each
(section, candidate-CRE)pair together and re-sorts the shortlist. C.2 hands up a raw relevance logit per candidate:+1.5for a good match,−11.3for a bad one. Perfect for ordering — but it is not a probability.+1.5does not mean "82% sure this is the right CRE," and cross-encoders are systematically over-confident, so evensigmoid(z)lies.The problem: Week 6's decision — auto-link vs. send to a human — is a threshold on confidence ("link if ≥ 90%"). That threshold is only safe if 90% really means 90%. A raw score cannot be thresholded honestly.
This PR's role: build the calibration step (C.3). Learn a single scalar temperature
Tby minimising negative-log-likelihood on the golden set, map every rerank logit to a real probabilityp = sigmoid(z / T), and prove the result honest with Expected Calibration Error < 0.10. Dividing every logit by the sameTnever changes the ranking, so calibration cannot hurt C.1/C.2 recall — it only makes the confidence trustworthy.One line: the bi-encoder ranks the whole map (W3), the cross-encoder re-ranks the shortlist (W4), and calibration turns the top score into an honest probability (W5) that the W6 decision engine can threshold on.
Scope: 1 new package (1 module + 1 test) + harness wiring for the ECE gate.
UpdateDetection(the other W5 schema deliverable) already exists inschemas.py— verified, nothing to add. No frontend, no migration, no behaviour change to OpenCRE proper.What changed
calibration/temperature.py(new),calibration/__init__.py(new)TemperatureScaler(T)maps a logit top = sigmoid(z/T)(.probabilitysingle /.calibratebatch);fit_temperaturelearns the single scalarTby minimising NLL (scipy.optimize.minimize_scalar, bounded), guarding single-class data (DegenerateLabelsError);expected_calibration_erroris the 10-bin ECE gate;negative_log_likelihoodis the fit objective (exposed for testing/reporting). Model-free (numpy +scipy.special.expit) so it stays import-light and hermetically testable — mirrors the C.1embed_fn/ C.2score_fnseams.CALIBRATOR_NAMEaudit tag (mirrorsRETRIEVER_NAME/RERANKER_NAME).evaluate_librarian.pyreport_calibrationbuilds a(logit, label)set from the live C.1→C.2 pipeline over the positive + hard_negative slices (top-1 rerank logit; label1iff top-1 is an expected CRE — hard_negatives supply the0class), fitsT, and reports ECE at T=1 vs. the fitted T with the < 0.10 gate._build_live_pipelineextracted so recall and calibration share one hub + model load. Offline path unchanged (still honest: no live number is faked).__init__.pytemperature_test.py(new)Tcool/heat, ranking preserved byT, NLL hand-check, fit recovers a knownTand reduces NLL vs. T=1, ECE on perfectly-/mis-calibrated data + a hand-checked two-bin value + thep == 1.0edge, and every guard (single-class, non-binary, length mismatch, empty,n_bins < 1, non-positive/non-finiteT).How the pieces connect
flowchart TB audit["RetrievalAudit from C.2 (W4)<br/>reranked[] with score_rerank (raw logit z)"] subgraph FIT["fit once on the golden set"] g["(logit, label) pairs<br/>positive + hard_negative slices"] nll["fit_temperature: argmin NLL(T)"] Tstar["T* (one scalar)"] ece["expected_calibration_error<br/>gate: ECE < 0.10"] g --> nll --> Tstar --> ece end subgraph APPLY["apply per candidate"] z["top-1 rerank logit z"] p["p = sigmoid(z / T*)<br/>honest probability"] z --> p end audit --> g audit --> z Tstar -.-> p p --> dec["W6 decision engine<br/>p >= τ -> LinkProposal ; else ReviewItem"]Results
What is intentionally not here
pinto aLinkProposal(≥ τ) vs. aReviewItem, and writing the calibratedconfidenceintoProposedLink. This PR produces the honest probability; W6 thresholds it.Tfor the live decision path (loaded by the W6 engine).--use_live_embeddingsrun.How to verify locally