Repository navigation
Conversation
`1 - exp(-x^2/(2*sigma^2))` cancels for small `x/sigma` and is `0` once the exponent is below `2^-53`. The fixtures are now generated from the textbook formula in extended precision; the Distributions.jl ones had the same cancellation.
Coverage Report
The above coverage report was generated for the changes in this PR. |
0PrashantYadav0
left a comment
There was a problem hiding this comment.
Same x/sigma issue as #15798 (comment).
| s2 = pow( sigma, 2.0 ); | ||
| return 1.0 - exp( -pow( x, 2.0 ) / ( 2.0 * s2 ) ); | ||
| return -expm1( -pow( x, 2.0 ) / ( 2.0 * s2 ) ); |
There was a problem hiding this comment.
| s2 = pow( sigma, 2.0 ); | |
| return 1.0 - exp( -pow( x, 2.0 ) / ( 2.0 * s2 ) ); | |
| return -expm1( -pow( x, 2.0 ) / ( 2.0 * s2 ) ); | |
| return -expm1( -0.5 * pow( x / sigma, 2.0 ) ); |
Squaring x and sigma separately overflows/underflows: cdf( 1e200, 1e200 ) and cdf( 1e-200, 1e-200 ) return NaN, cdf( 1e150, 1e200 ) returns 0. Taking the ratio first (as in weibull/cdf) stays within 2 ULP of mpmath for sigma in [1e-200, 1e200].
Same in lib/factory.js:75; drop s2 in both.
| s2 = stdlib_base_pow( sigma, 2.0 ); | ||
| return 1.0 - stdlib_base_exp( -stdlib_base_pow( x, 2.0 ) / ( 2.0 * s2 ) ); | ||
| return -stdlib_base_expm1( -stdlib_base_pow( x, 2.0 ) / ( 2.0 * s2 ) ); |
There was a problem hiding this comment.
| s2 = stdlib_base_pow( sigma, 2.0 ); | |
| return 1.0 - stdlib_base_exp( -stdlib_base_pow( x, 2.0 ) / ( 2.0 * s2 ) ); | |
| return -stdlib_base_expm1( -stdlib_base_pow( x, 2.0 ) / ( 2.0 * s2 ) ); | |
| return -stdlib_base_expm1( -0.5 * stdlib_base_pow( x / sigma, 2.0 ) ); |
Same as lib/main.js:77; also drop double s2;.
| # Small `x`: | ||
| x = exp10.( range( -150.0, stop = -1.0, length = 1000 ) ); | ||
| sigma = ( rand( 1000 ) .* 10.0 ) .+ 0.1; | ||
| gen( x, sigma, "small_x.json" ); |
There was a problem hiding this comment.
| # Small `x`: | |
| x = exp10.( range( -150.0, stop = -1.0, length = 1000 ) ); | |
| sigma = ( rand( 1000 ) .* 10.0 ) .+ 0.1; | |
| gen( x, sigma, "small_x.json" ); | |
| # Small `x`: | |
| sigma = exp10.( ( rand( 1000 ) .* 400.0 ) .- 200.0 ); | |
| x = sigma .* exp10.( range( -150.0, stop = -1.0, length = 1000 ) ); | |
| gen( x, sigma, "small_x.json" ); |
With sigma in [0.1, 10.1] this fixture can't catch the issue above. With this range the current code fails 415/1000 points at 2 ULP (115 NaN), the ratio version none.
| function gen( x, sigma, name ) | ||
| z = Array{Float64}( undef, length(x) ); | ||
| for i in eachindex(x) | ||
| z[ i ] = cdf( Rayleigh( sigma[i] ), x[i] ); | ||
| z = setprecision( BigFloat, 2048 ) do | ||
| Float64.( 1 .- exp.( .-( BigFloat.( x ) .^ 2 ) ./ ( 2 .* BigFloat.( sigma ) .^ 2 ) ) ) | ||
| end |
There was a problem hiding this comment.
| function gen( x, sigma, name ) | |
| z = Array{Float64}( undef, length(x) ); | |
| for i in eachindex(x) | |
| z[ i ] = cdf( Rayleigh( sigma[i] ), x[i] ); | |
| z = setprecision( BigFloat, 2048 ) do | |
| Float64.( 1 .- exp.( .-( BigFloat.( x ) .^ 2 ) ./ ( 2 .* BigFloat.( sigma ) .^ 2 ) ) ) | |
| end | |
| function gen( x, sigma, name ) | |
| # The CDF is evaluated in extended precision using the textbook formula `1 - exp(-x^2 / (2*sigma^2))`, as subtracting the exponential from `1` in double precision cancels for small `x/sigma`... | |
| z = setprecision( BigFloat, 2048 ) do | |
| Float64.( 1 .- exp.( .-( BigFloat.( x ) .^ 2 ) ./ ( 2 .* BigFloat.( sigma ) .^ 2 ) ) ); | |
| end |
And drop the docstring paragraph at line 26. Matches kgryte's follow-up commits to runner.jl in #15774.
Squaring `x` and `sigma` separately overflows or underflows for parameters far from 1: `cdf( 1e200, 1e200 )` and `cdf( 1e-200, 1e-200 )` returned NaN and `cdf( 1e150, 1e200 )` returned 0. Evaluate `(x/sigma)^2` instead, as `weibull/cdf` does. The small-`x` fixture now draws `sigma` from [1e-200, 1e200] so it covers those cases; the previous code fails 401 of its points.
36e9dcb to
8b3cf42
Compare
|
@0PrashantYadav0 thanks — applied in 8b3cf42:
Your three cases now give |
Description
This pull request:
stats/base/dists/rayleigh/cdfas-expm1( -x^2 / (2*sigma^2) )instead of1 - exp( -x^2 / (2*sigma^2) )(JS, factory and C).Subtracting the exponential from
1cancels whenever the CDF is small, and gives0oncex^2/(2*sigma^2) < 2^-53:rayleigh/logcdfalready usesexpm1for the same reason.Fixtures and tests:
runner.jlnow evaluates the textbook formula in 2048-bitBigFloatinstead ofDistributions.cdf. Distributions has the same1 - expcancellation, which is why the old fixtures passed at 0 ULP.Distributionsis dropped fromREQUIRE, and the three fixtures are regenerated with the same input ranges.developis off by up to ~2.9 million ULP onmedium_scale.json. With this change all fixtures pass at 2 ULP in JS and native, so the tolerances go from 0 to 2.small_x.jsonwithxlog-spaced over[1e-150, 0.1].develop, 1,689 of the 4,011 assertions intest.cdf.jsfail. With this change they all pass.Related Issues
None.
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 scanning
stats/base/distsfor1 - exp(...).@stdlib-js/reviewers