Skip to content

fix: scale beta_reg's continued-fraction bound with its parameters - #417

Closed
agene0001 wants to merge 1 commit into
statrs-dev:mainfrom
agene0001:fix/beta-reg-robustness
Closed

fix: scale beta_reg's continued-fraction bound with its parameters#417
agene0001 wants to merge 1 commit into
statrs-dev:mainfrom
agene0001:fix/beta-reg-robustness

Conversation

@agene0001

Copy link
Copy Markdown
Contributor

Stacked on #416 (whose accurate ln_gamma sets this PR's error floor); review only the last commit until that merges.

The Lentz recurrence in checked_beta_reg was capped at a fixed 140 iterations and silently returned whatever it had reached. The recurrence is slowest at the centre of the distribution, where the worst case over x grows like 5·min(a,b)^(1/3) (measured over a,b ∈ [1e2, 1e18] × 4001 values of x), so past min(a,b) ≈ 1.5e4 the answer was simply wrong. Against the exact identity I_½(a,a) = ½:

a = b value returned rel. error
1e5 0.49999969504 6.1e-7
1e6 0.49121972700 1.8e-2
1e7 0.21285001452 5.7e-1

Binomial::new(0.5, 2e6).cdf(1e6) returned 0.4916 against a true 0.50028 — this caps StudentsT, FisherSnedecor, and Beta cdfs too.

The bound now scales as 8·min(a,b)^(1/3) with headroom, clamped to bound worst-case work at ~10 ms. The loop still exits on convergence, so ordinary calls are unchanged (beta_reg(2.5, 2.5, 0.5): 150 ns vs 145 ns).

Plus four robustness fixes found by probing the parameter space rather than sweeping accuracy:

  • an underflowed prefix short-circuits to the corresponding endpoint — exact, and avoids 0.0 × h, which is NaN whenever the recurrence overflowed (beta_reg(1e300, 1e-300, 0.5) was NaN before, on both sides of the bound change);
  • x == 0 / x == 1 return 0/1 directly instead of via the symmetry test, which mapped x == 0 to 1.0 once a + b overflowed;
  • the symmetry threshold (a+1)/(a+b+2) is computed scaled when a + b overflows, otherwise it collapses to 0 and sends every x down the transformed branch;
  • the result stays in [0, 1] (it's a probability; a = b = 1e20 previously returned −2.56), falling back to the concentrated-limit step function if the truncated recurrence went non-finite.

Regression tests use exact identities needing no reference data — I_½(a,a) = ½ and I_x(a,b) + I_{1−x}(b,a) = 1 (both fail on the old fixed bound) — plus a 12×12×6 extreme-parameter grid asserting the result stays a finite probability.

🤖 Generated with Claude Code

The Lentz recurrence in `checked_beta_reg` was capped at a fixed 140 iterations
and silently returned whatever it had reached. It is slowest at the centre of the
distribution, where the worst case over `x` grows like `5 * min(a, b).cbrt()`, so
past `min(a, b) ~ 1.5e4` the answer was simply wrong. Against the exact identity
`I_{1/2}(a, a) == 1/2`:

    a = b = 1e5    0.49999969504   relative error 6.1e-7
    a = b = 1e6    0.49121972700                  1.8e-2
    a = b = 1e7    0.21285001452                  5.7e-1

`Binomial::new(0.5, 2e6).cdf(1e6)` returned 0.4916 against a true 0.50028.

The bound now scales as `8 * min(a, b).cbrt()`, measured to cover the worst case
over `x` with headroom, clamped to bound the work at roughly 10 ms. The loop still
exits on convergence, so ordinary calls are unaffected: `beta_reg(2.5, 2.5, 0.5)`
is 150 ns against 145 ns, and `beta_reg(1, 1, 0.3)` is unchanged.

Three robustness fixes alongside it, all found by probing the parameter space
rather than by sweeping accuracy:

  * an underflowed prefix now short-circuits to the corresponding endpoint. The
    result is `bt * h / a` with `h` of order one, so this is exact - and it avoids
    forming `0.0 * h`, which is NaN whenever the recurrence overflowed
    (`beta_reg(1e300, 1e-300, 0.5)` was NaN on both sides of this change before).
  * `x == 0` and `x == 1` return 0 and 1 directly. They used to depend on the
    symmetry test, which mapped `x == 0` to `1.0` once `a + b` overflowed.
  * the symmetry threshold `(a + 1) / (a + b + 2)` is computed scaled when
    `a + b` overflows, since otherwise it collapses to zero and sends every `x`
    down the transformed branch.
  * the result is kept in `[0, 1]`, and falls back to the concentrated-limit step
    function if the truncated recurrence produced something non-finite. `I_x` is a
    probability and callers such as `Binomial::cdf` are contractually so;
    `a = b = 1e20` previously returned -2.56.

Regression tests use two exact identities that need no reference data -
`I_{1/2}(a, a) == 1/2` and `I_x(a, b) + I_{1-x}(b, a) == 1` - plus a 12x12x6
parameter grid asserting the result stays a finite probability. Both identity
tests fail on the old fixed bound.
@YeungOnion
YeungOnion force-pushed the fix/beta-reg-robustness branch from 32b6618 to 1c199a7 Compare August 26, 2026 17:23
@coderabbitai

coderabbitai Bot commented Aug 26, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.52%. Comparing base (b152c47) to head (1c199a7).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   95.49%   95.52%   +0.02%     
==========================================
  Files          65       65              
  Lines       15403    15457      +54     
==========================================
+ Hits        14709    14765      +56     
+ Misses        694      692       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@YeungOnion

Copy link
Copy Markdown
Contributor

Closing — #456 has since landed a full rewrite of checked_beta_reg (Temme's uniform asymptotic expansion for the large-parameter prefactor, a 100k-iteration continued fraction that returns ConvergenceFailed instead of silently truncating, plus the endpoint/overflow handling this PR added). Every problem here is covered, more rigorously and with reference-validated accuracy. No remaining gap for this PR to fill.

@YeungOnion YeungOnion closed this Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants