Repository navigation
Conversation
`1 - p` rounds away the low-order bits of `p`. For `p < 2^-53` the function returned `-1`, outside the support, and for small `lambda` it was off by more than 10%.
Coverage Report
The above coverage report was generated for the changes in this PR. |
| return PINF; | ||
| } | ||
| return ceil( -ln( 1.0-p ) / lambda ) - 1.0; | ||
| return ceil( -log1p( -p ) / lambda ) - 1.0; |
There was a problem hiding this comment.
| return ceil( -log1p( -p ) / lambda ) - 1.0; | |
| return max( 0.0, ceil( -log1p( -p ) / lambda ) - 1.0 ); |
Should this clamp at 0 like geometric/quantile? -log1p( -p ) / lambda can still underflow to 0, so quantile( 5.0e-324, 2.0 ) and quantile( 1.0e-300, 1.0e300 ) return -1. The clamp would also settle the p = 0 question in the description. Same in lib/factory.js:73 and src/main.c:43 (plus @stdlib/math/base/special/max in manifest.json).
There was a problem hiding this comment.
Yes, clamped with max( 0.0, ... ) in main.js, factory.js and src/main.c, with @stdlib/math/base/special/max added to all three manifest confs (571b8e9). New tests in all three files cover p = 0, quantile( 5.0e-324, 2.0 ) and quantile( 1.0e-300, 1.0e300 ), which returned -1 before and now return 0. That also answers the p = 0 question, so I dropped it from the description.
| t.end(); | ||
| }); | ||
|
|
||
| tape( 'the created function evaluates the quantile for small `p`', function test( t ) { |
There was a problem hiding this comment.
| tape( 'the created function evaluates the quantile for small `p`', function test( t ) { | |
| tape( 'the returned function evaluates the quantile for small `p`', function test( t ) { |
Match the other test names in this file.
There was a problem hiding this comment.
Renamed in 571b8e9.
`-log1p( -p ) / lambda` can still underflow to `0` (e.g. `p = 5e-324`, `lambda = 2`), giving `ceil( 0 ) - 1 = -1`, outside the support. Clamping at `0` like `geometric/quantile` fixes that and also returns `0` at `p = 0`. Signed-off-by: Abhist17 <abhistkamle.170903@gmail.com>
Description
This pull request:
-log1p( -p )instead of-ln( 1.0 - p )instats/base/dists/planck/quantile(JS, factory and C).0, likegeometric/quantile, sop = 0and inputs where-log1p( -p ) / lambdaunderflows (e.g.quantile( 5.0e-324, 2.0 )) return0instead of-1.Forming
1 - pin double precision rounds away the low-order bits ofp. That gives two kinds of wrong answer:SciPy's
planck.ppfalready useslog1p(-q), so the existing Python fixtures pass unchanged with exact equality.Tests: a new "small
p" test intest.quantile.js,test.factory.jsandtest.native.jscoversp = 1e-20and1e-300(expected0) and two small-lambdacases (expected100, checked against 2048-bitBigFloat). Four of those assertions fail ondevelop.Related Issues
None. Same bug as #15762 (
weibull/quantile) and #15763 (exponential/quantile).Questions
No.
Other
No.
Checklist
AI Assistance
If you answered "yes" above, how did you use AI assistance?
Disclosure
This PR was written primarily by Claude Code. I found the bug by reading the
stats/base/distsquantile functions for1 - pcancellation.@stdlib-js/reviewers