Repository navigation
Conversation
…p the lower tail `mu + sigma*sqrt(2)*erfinv(2p-1)` loses the low-order digits of small `p` when forming `2p-1`, and for `p < 2^-54` that difference rounds to `-1`, so the function returned `-Infinity` (e.g., `quantile( 1e-20, 0, 1 )`, whose value is about `-9.26`). `lognormal/quantile` inherits the problem and returned `0`. Evaluate `mu - sigma*sqrt(2)*erfcinv(2p)` instead, as `binomial/quantile` and `poisson/quantile` already do, and add fixtures for both tails generated in 2048-bit precision.
Contributor
Coverage Report
The above coverage report was generated for the changes in this PR. |
5 of 7 tasks
This branch has not been deployed
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:
stats/base/dists/normal/quantile(JS,factory, and C) returning-Infinity, or losing digits, in the lower tail.lognormal/quantile, which calls it, inherits the fix.The implementation evaluates
mu + sigma*sqrt(2)*erfinv(2p-1). Forming2p-1throws away the low-order digits of smallp, and oncep < 2^-54the result is exactly-1, soerfinvreturns-Infinity. Before that point, the lower tail already loses accuracy (relative error ~1e-16/pin the probability).The fix evaluates the equivalent
mu - sigma*sqrt(2)*erfcinv(2p), which is whatbinomial/quantileandpoisson/quantilealready do for their normal approximations (-erfcinv( 2.0*p ) * SQRT2).2pis exact, anderfcinvof a small argument is accurate. The upper tail isn't hurt: there,erfcinvreceives an argument near2, which it handles as2 - ywithout cancellation. The method is documented in the JSDoc and C doc comments.Related Issues
No.
Questions
No.
Other
quantile( Normal( mu, sigma ), p )from Distributions.jl in 2048-bitBigFloat(an independent reference, not the formula used here) for 500 points in each tail.lower_tail.jsonhasplog-uniform in[1e-250, 1e-3], andupper_tail.jsonhas1 - plog-uniform in[1e-16, 1e-3].muis in[-10, 10]andsigmain[0.1, 10.1].develop, 472 of the 500 lower-tail points return-Infinity, and 499 assertions fail intest.quantile.js.developis at 96. That error is the cancellation inmu + sigma*zwhen the result is near zero, not the tail evaluation, and the tests allow 96. The existing R fixture test is unchanged and passes.1e-250becausemath/base/special/erfcinvitself loses accuracy below about1e-260(up to ~500 ULP near1e-300, against 2048-biterfcinv). That's a separate issue inerfcinv, which I'll look at on its own.test.quantile.js,test.factory.jsandtest.native.js, and in thelognormal/quantile,normal/ctor,lognormal/ctorandstats/fligner-testdependents.Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code. The bug was found by comparing every
stats/base/distsfunction against SciPy on random parameters.@stdlib-js/reviewers