Add percentile-bootstrap confidence intervals for aggregated metrics - #62
Add percentile-bootstrap confidence intervals for aggregated metrics#62ipezygj wants to merge 4 commits into
Conversation
A mean metric over N cases is a point estimate: a mean DSC of 0.85 over 20 cases and over 2,000 cases support very different conclusions, and validation studies routinely compare methods whose intervals overlap entirely. The Metrics Reloaded recommendations call for reporting variability alongside aggregates. - utility/uncertainty.py: percentile_bootstrap_ci for the mean of per-case values (NaN-ignored, consistent with the package's masked aggregations; seeded by default so reported intervals reproduce) and stats_with_ci, which appends ci95_low/ci95_high rows to a describe() summary - ProcessEvaluation.get_stats_res now reports stats_all with the interval rows; columns unchanged, existing consumers unaffected - tests: interval brackets the mean, determinism under seed, narrowing with N, agreement with the analytic normal interval on Gaussian data, NaN handling, degenerate inputs
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds seeded percentile-bootstrap confidence intervals, dataframe summary rows, overall evaluation integration, and tests for interval and summary behavior. ChangesUncertainty statistics
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProcessEvaluation
participant stats_with_ci
participant percentile_bootstrap_ci
ProcessEvaluation->>stats_with_ci: pass grouped_lab and excluded identifiers
stats_with_ci->>percentile_bootstrap_ci: compute metric intervals
percentile_bootstrap_ci-->>stats_with_ci: return confidence bounds
stats_with_ci-->>ProcessEvaluation: return summary statistics with interval rows
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@MetricsReloaded/processes/overall_process.py`:
- Line 692: Update the call to stats_with_ci in the surrounding process flow so
identifier columns such as case are excluded from confidence-interval
bootstrapping. Preserve the existing summary column if needed, calculate CI rows
only for metric columns, and leave identifier CI cells empty.
In `@MetricsReloaded/utility/uncertainty.py`:
- Around line 33-59: Update percentile_bootstrap_ci to reject invalid n_boot and
alpha values before resampling, ensuring n_boot is positive and alpha defines a
valid confidence interval with ordered quantiles. Then update the CI row-name
construction near the existing ci98 labeling to derive the label from the full
confidence level (for example, 1 - alpha) rather than truncating or rounding
97.5% to ci98.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 54299000-d415-460c-85bb-5fd1bcfc449f
📒 Files selected for processing (3)
MetricsReloaded/processes/overall_process.pyMetricsReloaded/utility/uncertainty.pytest/test_utility/test_uncertainty.py
…tifier columns - percentile_bootstrap_ci rejects n_boot < 1 and alpha outside (0, 1) instead of failing inside np.quantile or returning inverted bounds - interval row names preserve fractional confidence levels (alpha=0.025 -> ci97.5_low, previously mislabelled ci98) - stats_with_ci grew an exclude= option; get_stats_res passes the identifier columns so 'case'/'index' keep their describe() rows but no longer get a meaningless bootstrap interval - three new tests cover each point
Both bounds are quantiles of the resample distribution, so a single resample makes them the same number: percentile_bootstrap_ci(values, n_boot=1) returned (0.5, 0.5) on a sample spanning 0.1 to 0.9. The old check only asked whether n_boot was positive, so the input that produces the strongest possible claim -- an interval of exactly zero width -- was also the cheapest one to pass. Requires n_boot >= 2 and documents the sharper limit behind it: while n_boot < 2 / alpha, both bounds are decided by the most extreme draws alone (fewer than 40 resamples at the default alpha=0.05), so the interval comes out too narrow rather than merely noisy.
|
Pushed
Now requires The new test asserts the rejection rather than the old value. I checked that it discriminates by reverting the guard and re-running: 9 pass either way, that one fails without it. 10 tests green. Found this while auditing my other open PRs after a reviewer elsewhere caught the same shape in code of mine — a degenerate sample size that reached a confident number instead of an absent one. Sorry for the extra round on this one. |
1 participant
Why
A mean metric over N cases is a point estimate: a mean DSC of 0.85 over 20 cases and over 2,000 cases support very different conclusions, and validation studies routinely compare methods whose intervals overlap entirely. The Metrics Reloaded framework's recommendations call for reporting variability alongside aggregated performance — the library computes the aggregates but currently reports no uncertainty for them (no confidence interval or bootstrap machinery in the package).
What
MetricsReloaded/utility/uncertainty.py—percentile_bootstrap_ci(values): percentile-bootstrap CI for the mean of per-case metric values. NaN-ignored (consistent with the package's masked aggregations), seeded by default so reported intervals are reproducible run-to-run. Andstats_with_ci(df): adescribe()summary augmented withci95_low/ci95_highrows per metric column.ProcessEvaluation.get_stats_res()now producesstats_allwith the interval rows appended. Columns are unchanged, so existing consumers (incl.test_overall_process's column assertion) are unaffected — the change is additive rows only.test/test_utility/test_uncertainty.py): interval brackets the mean, determinism under the default seed, narrowing with N, agreement with the analytic normal interval on Gaussian data, NaN handling, degenerate inputs. 6/6 passing; the full suite shows the same 3 pre-existing failures as the untouched base on my environment (numpy/scipy drift, unrelated).Context
I work on eval-integrity tooling (statistical checks for benchmark and leaderboard claims — e.g. a published audit showing a clinical AUC of 0.978 corresponds to ~12% PPV at 1% prevalence, which is a Metrics Reloaded pitfall measured in the wild). Happy to extend this to per-label stats, paired method-comparison intervals, or whatever shape fits the framework's roadmap best.
Summary by CodeRabbit
New Features
Bug Fixes