Skip to content

fix: avoid overflow and duplicate computation - #14446

Merged
kgryte merged 5 commits into
developfrom
philipp/fix-propagation-2026-08-20
Aug 21, 2026
Merged

kgryte merged 5 commits into
developfrom
philipp/fix-propagation-2026-08-20

Conversation

@Planeshifter

@Planeshifter Planeshifter commented Aug 20, 2026 •

Copy link
Copy Markdown
Member

Description

What is the purpose of this pull request?

This pull request propagates a fix merged to develop between 2026-08-19 and 2026-08-20 to sibling packages exhibiting the same defect.

  • Intermediate overflow in distribution functions — 76d80cc4

    Fixes the same defect Athan patched in 76d80cc for stats/base/dists/wald/logpdf: squaring an unnormalized deviate, or squaring the scale parameter in isolation, before combining quotients, so intermediate products overflow or underflow and the function returns NaN for inputs whose true double-precision result is representable. Applied the same normalize-before-square regrouping to stats/base/dists/wald/pdf, stats/base/dists/normal/pdf, stats/base/dists/normal/logpdf, stats/base/dists/lognormal/pdf, stats/base/dists/lognormal/logpdf, stats/base/dists/truncated-normal/pdf, and stats/base/dists/halfnormal/logpdf, across their lib/main.js, lib/factory.js, and (where present) src/main.c implementations. Confirmed failures included wald/pdf( 1e-200, 1e-200, 1.0 ), normal/pdf( 1e200, 0.0, 1e200 ), lognormal/logpdf( 1.0, -1e200, 1e200 ), and halfnormal/logpdf( 1e200, 1e195 ), all previously NaN; existing R and Julia fixture assertions pass unchanged, with ordinary-input results differing by no more than a few ULP.

    Target packages:

    • stats/base/dists/wald/pdf
    • stats/base/dists/normal/pdf
    • stats/base/dists/normal/logpdf
    • stats/base/dists/lognormal/pdf
    • stats/base/dists/lognormal/logpdf
    • stats/base/dists/truncated-normal/pdf
    • stats/base/dists/halfnormal/logpdf

Related Issues

Does this pull request have any related issues?

No.

Questions

Any questions for reviewers of this pull request?

No.

Other

Any other information relevant to this pull request? This may include screenshots, references, and/or implementation notes.

Validation. Candidate sites were located by searching stats/base/dists/**/{lib/main.js,lib/factory.js,src/main.c} for premature-squaring shapes. Each surviving site was reviewed by two independent validation passes (defect reproduced from concrete inputs against an independently derived reference value), an adaptation pass producing the per-site patch, and a style-consistency pass. Patched implementations were checked against the pre-patch implementations over a grid of ordinary inputs (maximum relative deviation 1.1e-13, occurring in a deep tail well inside the package's fixture tolerance), and the C implementations were compiled and compared against their JavaScript counterparts. All 20,275 assertions in the affected packages' test suites pass.

Deliberately excluded.

  • stats/base/dists/wald/mgf — same overflow class, but no regrouping analogous to the source fix repairs every failing input; the correct repair requires a branched or conjugate reformulation, which is a maintainer decision rather than a propagation.
  • stats/base/dists/f/pdf — regrouping the rational term removes the NaN, but the returned value remains wrong because the inaccuracy originates in ibeta_derivative.js, outside the target files.
  • stats/base/dists/rayleigh/pdf and stats/base/dists/rayleigh/logpdf — the defect is present, but every overflow-safe reordering shifts results beyond the 2-ULP (respectively 15-EPS) agreement those packages assert against their Julia fixtures, so landing the fix would require changing fixtures or tolerances.
  • strided/base/map-by2 — the source commit 4e9f4337 corrected the public entry points, but four private implementation files (lib/accessors.js, lib/map.js, lib/accessors.ndarray.js, lib/map.ndarray.js) still carry the stale singular-input description. These are in the originating package rather than a sibling, so they are left for a follow-up.
  • string/format — a second pattern from 10dda530 (hyphenating "floating point" used as a compound adjective) applied cleanly to three lines of that README, but including it pulls the README into Run changed examples, which then fails on four pre-existing, unrelated example expectations. Rather than widen this PR, that change is dropped; the pre-existing errors are reported below.
  • Sites whose fix would cascade into test, fixture, or cross-package changes, and adaptive fixes that could not be confined to a single file.

Unrelated pre-existing defects observed in lib/node_modules/@stdlib/string/format/README.md (not addressed here; reported for a separate PR). Actual values verified against string/format:

Line Documented Actual
124 '3.14159' '3.14159 (shortest representation)'
177 ' 2' ' 2'
~220 'a b ' 'a b '
~223 ' a b' ' a b'

Checklist

Please ensure the following tasks are completed before submitting this pull request.

AI Assistance

When authoring the changes proposed in this PR, did you use any kind of AI assistance?

  • Yes
  • No

If you answered "yes" above, how did you use AI assistance?

  • Code generation (e.g., when writing an implementation or fixing a bug)
  • Test/benchmark generation
  • Documentation (including examples)
  • Research and understanding

Disclosure

If you answered "yes" to using AI assistance, please provide a short disclosure indicating how you used AI assistance. This helps reviewers determine how much scrutiny to apply when reviewing your contribution. Example disclosures: "This PR was written primarily by Claude Code." or "I consulted ChatGPT to understand the codebase, but the proposed changes were fully authored manually by myself.".

This PR was produced by an automated Claude Code routine that reviews fixes merged to develop in the preceding 24 hours, searches for sibling packages exhibiting the same defect, and proposes the equivalent fix. Defect presence was confirmed at each site from concrete failing inputs, and every patch was verified against the package's existing fixtures before inclusion; sites requiring human judgment were dropped rather than patched. Opened as a draft for maintainer review.


@stdlib-js/reviewers

Propagates fix from 76d80cc ("fix: avoid overflow in
`stats/base/dists/wald/logpdf`") to sibling distribution functions
exhibiting the same defect: normalizing a deviate by its scale
parameter only after squaring, so that intermediate products overflow
or underflow and the function returns NaN where the true
double-precision value is representable.

Normalizing before squaring and regrouping the surrounding quotients
repairs, e.g., wald/pdf( 1e-200, 1e-200, 1.0 ), normal/pdf( 1e200,
0.0, 1e200 ), lognormal/logpdf( 1.0, -1e200, 1e200 ) and
halfnormal/logpdf( 1e200, 1e195 ), each of which previously returned
NaN. Ordinary inputs are unchanged to within a few ULP, and all
existing R and Julia fixture assertions continue to pass.

Affected packages: wald/pdf, normal/pdf, normal/logpdf,
lognormal/pdf, lognormal/logpdf, truncated-normal/pdf,
halfnormal/logpdf.
@Planeshifter
Planeshifter force-pushed the philipp/fix-propagation-2026-08-20 branch from ef8b65e to b7ae2b1 Compare August 20, 2026 14:19
@stdlib-bot stdlib-bot added the Statistics Issue or pull request related to statistical functionality. label Aug 20, 2026
@stdlib-bot

stdlib-bot commented Aug 20, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Report

Package Statements Branches Functions Lines
stats/base/dists/halfnormal/logpdf $\\color{red}305/307$
$\\color{green}+0.00\\%$
$\\color{red}19/20$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{red}305/307$
$\\color{green}+0.00\\%$
stats/base/dists/lognormal/logpdf $\\color{green}319/319$
$\\color{green}+0.00\\%$
$\\color{green}23/23$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}319/319$
$\\color{green}+0.00\\%$
stats/base/dists/lognormal/pdf $\\color{green}315/315$
$\\color{green}+0.00\\%$
$\\color{green}23/23$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}315/315$
$\\color{green}+0.00\\%$
stats/base/dists/normal/logpdf $\\color{green}304/304$
$\\color{green}+0.00\\%$
$\\color{green}23/23$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}304/304$
$\\color{green}+0.00\\%$
stats/base/dists/normal/pdf $\\color{green}306/306$
$\\color{green}+0.00\\%$
$\\color{green}25/25$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}306/306$
$\\color{green}+0.00\\%$
stats/base/dists/truncated-normal/pdf $\\color{red}193/248$
$\\color{red}-0.23\\%$
$\\color{green}3/3$
$\\color{green}+0.00\\%$
$\\color{red}0/2$
$\\color{green}+0.00\\%$
$\\color{red}193/248$
$\\color{red}-0.23\\%$
stats/base/dists/wald/pdf $\\color{green}360/360$
$\\color{green}+0.00\\%$
$\\color{green}33/33$
$\\color{green}+0.00\\%$
$\\color{green}4/4$
$\\color{green}+0.00\\%$
$\\color{green}360/360$
$\\color{green}+0.00\\%$

The above coverage report was generated for the changes in this PR.

Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/factory.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/factory.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/main.js Outdated
@kgryte
kgryte marked this pull request as ready for review August 21, 2026 00:07
@kgryte
kgryte requested a review from a team August 21, 2026 00:07
@stdlib-bot stdlib-bot added the Needs Review A pull request which needs code review. label Aug 21, 2026
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/lib/factory.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/lib/factory.js Outdated
Addresses review feedback on the propagation of 76d80cc: replace
`pow( x, 2.0 )` with `abs2( x )` when squaring standardized values in
the JavaScript implementations. C implementations retain
`stdlib_base_pow` to avoid manifest dependency changes.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbLWivyEpdcK8DSErG9UjP
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/factory.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/src/main.c Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/lib/factory.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/lib/main.js Outdated
…dists/*`

Addresses review feedback: replace computed `-0.5 * ln( 2.0 * PI )` with
the `LN_TWO_PI` constant in `lognormal/logpdf` and, following the same
principle, replace `sqrt( 2π )` prefactor computations with the
`SQRT_TWO_PI` constant in `normal/pdf`, `lognormal/pdf`, and
`truncated-normal/pdf` JavaScript implementations.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbLWivyEpdcK8DSErG9UjP
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/src/main.c Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/pdf/src/main.c Outdated
Addresses review feedback: mirror the JavaScript changes in the C
implementations of `normal/pdf`, `normal/logpdf`, `lognormal/pdf`, and
`lognormal/logpdf` by using `stdlib_base_abs2` for squaring and the
`LN_TWO_PI`/`SQRT_TWO_PI` constants, with corresponding manifest
dependency updates.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbLWivyEpdcK8DSErG9UjP
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/main.js Outdated
Comment thread lib/node_modules/@stdlib/stats/base/dists/lognormal/logpdf/lib/factory.js Outdated
…logpdf`

Addresses review feedback: compute `ln( x )` once and reuse via a
temporary variable, mirroring the C implementation.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DbLWivyEpdcK8DSErG9UjP
@kgryte kgryte changed the title fix: propagate overflow fixes across stats/base/dists fix: avoid overflow and duplicate computation Aug 21, 2026
@kgryte
kgryte merged commit f51d7fe into develop Aug 21, 2026
12 checks passed
@kgryte
kgryte deleted the philipp/fix-propagation-2026-08-20 branch August 21, 2026 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Statistics Issue or pull request related to statistical functionality.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants