Closed
Bug 897634
Opened 11 years ago
Closed 8 years ago
Fix Math.expm1 when !HAVE_EXPM1
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: jorendorff, Assigned: arai)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
(deleted),
patch
|
jorendorff
:
review+
|
Details | Diff | Splinter Review |
Two issues:
1. Precision when the argument is > 0.00001 but still smallish
The current code computes exp(x)-1 when |x| >= 0.00001.
This loses some bits. The worst cases are:
js> Math.expm1(1e-5)
0.000010000050000166668 # system expm1
0.000010000050000069649 # exp(x)-1
js> Math.expm1(-1e-5)
-0.000009999950000166666 # system expm1
-0.000009999950000172397 # exp(x)-1
I'm pretty sure we can safely use that approximation when exp(x) is outside the range (1/2, 2), that is, |x| >= log(2) ~= 0.69314.
js> Math.expm1(0.69315)
1.0000056388880587 # system expm1
1.0000056388880587 # exp(x) - 1
but that's a much bigger range where we'll need to use a series approximation.
2. Monotonicity. This one is a surprise to me.
In bug 717379 comment 76, 4esn0k notes:
> with current algorithm for expm1 (!HAVE_EXPM1), expm1 is not monotonic
> Math.expm1(-1e-2) === -0.009950166250831893
> Math.expm1(-0.009999999999999998) === -0.009950166250831945
> so
> Math.expm1(-1e-2) > Math.expm1(-0.009999999999999998)
These arguments are outside the ±0.00001 threshold, so the non-monotonicity is happening in the exp(x) - 1 part of the range. So... I guess this means exp() itself is not monotonic on 4esn0k's platform. It's hard to guard against that.
The Taylor series approximation we use near 0 is monotonic if the C++ stack provides monotonic multiplication and addition.
@Jason Orendorff, when i tested it, i used a code from https://bugzilla.mozilla.org/attachment.cgi?id=779974&action=diff
where the threshold is 1e-2, not 0.00001
actually, the test should check, that:
Math.expm1(-threshold - Math.ulp(threshold)) <=
Math.expm1(-threshold) <=
Math.expm1(-threshold + Math.ulp(threshold)) <=
Math.expm1(threshold - Math.ulp(threshold)) <=
Math.expm1(threshold) <=
Math.expm1(threshold + Math.ulp(threshold))
Reporter | ||
Comment 3•11 years ago
|
||
Ah. Thanks, 4esn0k! This makes more sense. The two issues are then related after all.
Comment 4•11 years ago
|
||
Is it not possible to just use the fdlibm version of expm1? That's at least what's proposed in the ECMAScript spec.
Updated•10 years ago
|
Assignee: general → nobody
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 5•8 years ago
|
||
Now Math.expm1 is using fdlibm expm1, so we just need to add testcase in comment #2 and make sure it satisfies the requirement.
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → arai.unmht
Assignee | ||
Comment 6•8 years ago
|
||
it satisfies the comment #0 and comment #2 for several numbers, on OSX 64bit, and it should be same for other archs, as it's using same impl.
will post a patch to add testcase after testing on other archs.
Assignee | ||
Comment 7•8 years ago
|
||
about monotonicity (expm1-monotonicity.js), it passes all thresholds.
about precision (expm1-approx.js), it passes some more range, but result is still sloppy for large x.
Attachment #8767172 -
Flags: review?(jorendorff)
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8767172 [details] [diff] [review]
Add more testcase for Math.expm1.
Review of attachment 8767172 [details] [diff] [review]:
-----------------------------------------------------------------
Great work! Thank you.
Attachment #8767172 -
Flags: review?(jorendorff) → review+
Pushed by arai_a@mac.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ffc6d04b8cd7
Add more testcase for Math.expm1. r=jorendorff
Comment 10•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox50:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•