Repository navigation
feat: Benjamini–Hochberg FDR control for the point detector (0.5.0) - #54
Conversation
`scan`/`explain` gain `--fdr Q`: per-column false-discovery-rate control. Each cell's modified z-score becomes a two-sided p-value (consistent-σ standardized deviation, ≈N(0,1) under the null — not robustz's display-scaled modz) and the fixed point_threshold is replaced by the Benjamini–Hochberg step-up cutoff, bounding the expected proportion of false flags at Q. Opt-in (None default → unchanged behavior); the level is part of the config_version fingerprint (pfdr=). New ax_detect::fdr module: two_sided_p (erfc) + benjamini_hochberg (deterministic sort + step-up), property/exact tested. Honest scope: FDR is a CORRECTNESS control, not a volume knob. It replaces an arbitrary fixed cutoff with a principled, multiplicity-aware error-rate guarantee. On the real 127k-row MSHA parquet it flags MORE point cells than the old threshold (40,079 vs 32,893 at q=0.05) — those cells are genuinely significant; the fixed z>3.5 cutoff was stringent in an uncalibrated way. Capping volume is a separate concern (column scoping + planned severity/top-N). Gates: proptest + cargo-mutants 0-missed on fdr.rs/point.rs/config.rs (point 26 caught/1 unviable) and main.rs (54 caught/5 unviable). Includes a test pinning the (x-center) sign and one showing the same outlier is significant in a small column but not a large one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 4 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
✨ 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 |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
scan/explaingain--fdr Q— per-column Benjamini–Hochberg false-discovery-rate control forpoint.modz. Each cell's modified z-score becomes a two-sided p-value, and the fixedpoint_thresholdis replaced by a multiplicity-aware step-up cutoff that bounds the expected proportion of false flags atQ. Opt-in (omitted ⇒ unchanged behavior); the level is in theconfig_versionfingerprint (pfdr=).New
ax_detect::fdr:two_sided_p(normal tail viaerfc) +benjamini_hochberg(deterministic sort + step-up).Why this is #1 toward "full"
It replaces an arbitrary magic cutoff with a principled error-rate guarantee, and makes the discovery set adapt to how many cells were tested — a noise column stops contributing chance flags, and the same outlier can be significant in a small column yet not a large one.
Honest result (dogfooded on the real 127k-row MSHA parquet)
FDR is a correctness control, not a volume knob — and on heavy-tailed data it flags more, not fewer:
--fdr 0.05--fdr 0.01--fdr 0.001Those extra cells genuinely clear the FDR bar; the old fixed cutoff was stringent in an uncalibrated way (its effective per-cell p was ~2e-7). The takeaway: FDR makes findings defensible; capping their number is a separate lever (column scoping today, severity / top-N output scoping next). They compose: "top-N by score, among the FDR-significant set."
Calibration note
The p-value uses the consistent-σ standardized deviation
(x − center)/scale(≈N(0,1)), notrobustz's modified z-score — which folds in the display constantMODZ_Kand is therefore not unit-variance. Using the latter would mis-state every p-value.Gate
proptest+cargo-mutants0 missed on all four changed files:fdr.rs/point.rs/config.rs(point 26 caught / 1 unviable; BH+config covered) andmain.rs(54 caught / 5 unviable). Tests include the BH step-up property, the(x−center)sign, and multiplicity adaptation (same outlier significant at m=22, not at m=2101).🤖 Generated with Claude Code