Fix the iop=2 Temme expansion variable and the ind=2 wide-band truncation - #538
Conversation
…tion The iop=2 branch expanded c0/c1 in d00/d10 instead of z (NSWC gratio label 270), dropping the z-dependence of the expansion, and the ind=2 wide band discarded its computed 4-term expansion (label 230) in favour of the narrow-band 2-term form (label 280). Add reference-point regression tests for both branches.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #538 +/- ##
==========================================
+ Coverage 94.34% 94.49% +0.14%
==========================================
Files 14 14
Lines 3026 3016 -10
==========================================
- Hits 2855 2850 -5
+ Misses 171 166 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| @test gamma_inc(a, x, 1)[1] ≈ p rtol=5e-7 | ||
| @test gamma_inc(a, x, 1)[2] ≈ q rtol=5e-7 |
There was a problem hiding this comment.
Are these tolerances (and the ones below) as strict as possible, to increase the chances of detecting regressions?
| if l < 1.0 | ||
| p = c*(w - rt2pin*t/rta) | ||
| return (p, 1.0 - p) | ||
| else | ||
| q = c*(w + rt2pin*t/rta) | ||
| return (1.0 - q, q) | ||
| end |
There was a problem hiding this comment.
It seems this is the same code as in gamma_inc_temme_1 above (and possibly other places). Maybe could be worth moving to a separate function?
…d tolerances Extract the c/w/t -> (P,Q) combine into _gamma_inc_temme_combine, used by both gamma_inc_temme_1 and the ind=2 wide-band branch in _gamma_inc that previously duplicated it inline. Measured actual error against the mpmath references: wide-band tops out at 1.3e-4, narrow-band at 4.6e-5, so tighten rtol from 5e-4 to 2e-4/1e-4 respectively (~1.5-2x margin) to catch regressions sooner.
|
Both fair. Extracted the shared c/w/t -> (P,Q) combine into On tolerances: measured actual error against the mpmath references — wide-band tops out at 1.3e-4, narrow-band at 4.6e-5 — so tightened rtol from 5e-4 to 2e-4 / 1e-4 (roughly 1.5-2x margin over what's observed). Full suite still green. |
|
|
||
| External links: [DLMF 8.12.8](https://dlmf.nist.gov/8.12.8) | ||
| """ | ||
| # Combine the Temme series value `t` with the Gaussian-tail term `c*w` into (P, Q); |
There was a problem hiding this comment.
This removes the docstring (above) from gamma_inc_temme_1 (below). Moreover, this function shows up in more places in this file.
Fixes the docstring being pushed off gamma_inc_temme_1 by the helper's insertion point; also folds in gamma_inc_minimax and gamma_inc_temme, which had the same duplicated block.
|
Good catch on both — the helper landed between the docstring and And yes — |
2 participants
gamma_inc(a, x, 1) enters the iop==2 Temme branch for a >= 14 within |1 - x/a| <=
0.025/sqrt(a). That branch expands c0/c1 in the wrong variable (@evalpoly(d00, ...)
puts d00 in the variable slot), so the z-dependence is dropped entirely (NSWC gratio
label 270 is C0 = D00 + D0(1)Z + D0(2)Z^2). Results miss the documented 6-digit tier
by up to ~9% and no existing test enters the branch.
The ind=2 wide band (0.14/sqrt(a) < |1 - x/a| <= 0.4) had the same class of porting
error: the 4-term expansion (label 230) was computed and then discarded, falling back
to the narrow-band 2-term form (label 280) — up to ~1.6e-3 error against the 3-digit
tier.
Repro: gamma_inc(14.0, 13.906552106765321, 1) -> (0.4806, 0.5194); true P =
0.525615807496999.
Tests: 34 reference points across both bands plus ind=0 controls (68 assertions,
test/gamma_inc.jl); full suite passes.