Repository navigation
fix: avoid overflow and duplicate computation - #14446
Merged
Merged
Conversation
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
force-pushed
the
philipp/fix-propagation-2026-08-20
branch
from
August 20, 2026 14:19
ef8b65e to
b7ae2b1
Compare
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
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
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
…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
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
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
kgryte
reviewed
Aug 21, 2026
kgryte
reviewed
Aug 21, 2026
…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
approved these changes
Aug 21, 2026
stats/base/dists
4 of 7 tasks
This was referenced Sep 26, 2026
kgryte
pushed a commit
that referenced
this pull request
Sep 27, 2026
PR-URL: #15577 Reviewed-by: Athan Reines <kgryte@gmail.com> Closes: stdlib-js/metr-issue-tracker#1377 Ref: #14446 Ref: #15426
kgryte
pushed a commit
that referenced
this pull request
Sep 27, 2026
…l/pdf` PR-URL: #15576 Reviewed-by: Athan Reines <kgryte@gmail.com> Closes: stdlib-js/metr-issue-tracker#1376 Ref: #14446 Ref: #15426
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request propagates a fix merged to
developbetween 2026-08-19 and 2026-08-20 to sibling packages exhibiting the same defect.Intermediate overflow in distribution functions —
76d80cc4Fixes 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 returnsNaNfor inputs whose true double-precision result is representable. Applied the same normalize-before-square regrouping tostats/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, andstats/base/dists/halfnormal/logpdf, across theirlib/main.js,lib/factory.js, and (where present)src/main.cimplementations. Confirmed failures includedwald/pdf( 1e-200, 1e-200, 1.0 ),normal/pdf( 1e200, 0.0, 1e200 ),lognormal/logpdf( 1.0, -1e200, 1e200 ), andhalfnormal/logpdf( 1e200, 1e195 ), all previouslyNaN; 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/pdfstats/base/dists/normal/pdfstats/base/dists/normal/logpdfstats/base/dists/lognormal/pdfstats/base/dists/lognormal/logpdfstats/base/dists/truncated-normal/pdfstats/base/dists/halfnormal/logpdfRelated Issues
No.
Questions
No.
Other
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 theNaN, but the returned value remains wrong because the inaccuracy originates inibeta_derivative.js, outside the target files.stats/base/dists/rayleigh/pdfandstats/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 commit4e9f4337corrected 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 from10dda530(hyphenating "floating point" used as a compound adjective) applied cleanly to three lines of that README, but including it pulls the README intoRun 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.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 againststring/format:'3.14159''3.14159 (shortest representation)'' 2'' 2''a b ''a b '' a b'' a b'Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was produced by an automated Claude Code routine that reviews fixes merged to
developin 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