Skip to content

Restore integer truncation semantics from BPSER and BUP in the beta_inc translation - #542

Merged
andreasnoack merged 1 commit into
masterfrom
an/beta-inc-integer-truncation
Aug 17, 2026
Merged

Restore integer truncation semantics from BPSER and BUP in the beta_inc translation#542
andreasnoack merged 1 commit into
masterfrom
an/beta-inc-integer-truncation

Conversation

@andreasnoack

Copy link
Copy Markdown
Member

While looking into JuliaStats/StatsFuns.jl#228 (Student-t CDF/quantile performance), I audited the beta_inc translation against the original NSWC Fortran source (https://github.com/jacobwilliams/nswc, BRATIO and its subroutines). The Fortran routines rely on implicit typing — variables starting with IN are INTEGER unless declared REAL, and assigning a real expression to them truncates. The translation handles this correctly in several places (BRATIO's N = B0, BRCOMP's N = B0 - 1.0, BUP's K = R), but two spots were missed:

  1. BPSER (beta_inc_power_series): M = B0 - 1.0 truncates (BPSER declares only N as REAL), and DO 41 I = 1,M is an integer loop. The translation kept m as a Float64 and iterated for i = 1:m over a Float64 range, which constructs a TwicePrecision-backed StepRangeLen on every call — for a loop that typically runs a handful of times. The iteration count happens to coincide with the truncated Fortran semantics, so this is a pure performance fix: ~40 ns flat per call on the min(a,b) < 1 < max(a,b) < 8 branch (the cost is the range construction, not the iterations).

  2. BUP (beta_inc_diff): MU = ABS(EXPARG(1)) and K = EXPARG(0) truncate (BUP declares only L as REAL), so the scaling exponent is a whole number. This one has no measurable performance impact and produced bit-identical results in all tests; it is included for fidelity to the original.

Benchmarks (Apple Silicon, @belapsed, this branch vs current master; values bit-identical):

case master this PR
beta_inc(0.5, 2.5, 0.3103) 217 ns 179 ns (−17%)
StatsFuns.tdistcdf(5, 1.5) 223 ns 182 ns (−19%)
StatsFuns.tdistinvcdf(5, 0.95) 1004 ns 849 ns (−15%)
StatsFuns.tdistinvcdf(5, 1e-8) 1642 ns 1354 ns (−18%)

The quantile improves because beta_inc_inv evaluates beta_inc on every Newton iteration.

All beta_inc/betanc tests pass with results unchanged.

Disclosure: this PR was prepared by Claude Code at my direction; I have reviewed the changes and the benchmark methodology.

🤖 Generated with Claude Code

…routines

The NSWC Fortran routines rely on implicit typing, so variables starting
with I-N are INTEGER and assignments to them truncate. Two spots in the
beta_inc translation missed this:

- BPSER: M = B0 - 1.0 truncates (only N is declared REAL), and the
  following DO loop is an integer loop. The translation kept m as
  Float64 and iterated for i = 1:m over a Float64 range, constructing a
  TwicePrecision-backed StepRangeLen on every call. The iteration count
  happens to agree, so this is a pure performance fix (~40ns per call
  on the 1 < max(a,b) < 8 branch).

- BUP: MU = ABS(EXPARG(1)) and K = EXPARG(0) truncate (only L is
  declared REAL), so the exp scaling exponent is a whole number. No
  performance impact; restores fidelity to the original.

Results are bit-identical on the test suite and on spot checks.
@codecov

codecov Bot commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.49%. Comparing base (adbeb4b) to head (e7193aa).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #542   +/-   ##
=======================================
  Coverage   94.49%   94.49%           
=======================================
  Files          14       14           
  Lines        3016     3016           
=======================================
  Hits         2850     2850           
  Misses        166      166           
Flag Coverage Δ
unittests 94.49% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@andreasnoack
andreasnoack requested a review from devmotion August 17, 2026 08:29

@devmotion devmotion left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I checked the Fortran source and indeed this PR seems to fix inconsistencies with it.

@andreasnoack
andreasnoack merged commit 1d104a8 into master Aug 17, 2026
15 checks passed
@andreasnoack
andreasnoack deleted the an/beta-inc-integer-truncation branch August 17, 2026 12:21
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