Closed
Bug 897634
Opened 7 years ago
Closed 4 years ago
Fix Math.expm1 when !HAVE_EXPM1
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Not set
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking  Status  

firefox50    fixed 
People
(Reporter: jorendorff, Assigned: arai)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
6.78 KB,
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(1e5) 0.000010000050000166668 # system expm1 0.000010000050000069649 # exp(x)1 js> Math.expm1(1e5) 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(1e2) === 0.009950166250831893 > Math.expm1(0.009999999999999998) === 0.009950166250831945 > so > Math.expm1(1e2) > Math.expm1(0.009999999999999998) These arguments are outside the ±0.00001 threshold, so the nonmonotonicity 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 1e2, 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•7 years ago


Ah. Thanks, 4esn0k! This makes more sense. The two issues are then related after all.
Comment 4•7 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•6 years ago

Assignee: general → nobody
Assignee  
Comment 5•4 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•4 years ago

Assignee: nobody → arai.unmht
Assignee  
Comment 6•4 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•4 years ago


about monotonicity (expm1monotonicity.js), it passes all thresholds. about precision (expm1approx.js), it passes some more range, but result is still sloppy for large x.
Attachment #8767172 
Flags: review?(jorendorff)
Reporter  
Comment 8•4 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/mozillainbound/rev/ffc6d04b8cd7 Add more testcase for Math.expm1. r=jorendorff
Comment 10•4 years ago


bugherder 
https://hg.mozilla.org/mozillacentral/rev/ffc6d04b8cd7
Status: NEW → RESOLVED
Closed: 4 years ago
statusfirefox50:
 → fixed
Resolution:  → FIXED
Target Milestone:  → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•