-
Notifications
You must be signed in to change notification settings - Fork 113
Fix exponential() #226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix exponential() #226
Conversation
eaea933
to
a6eeebf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks generally good.
Fix various issues in exponential() like: - ** 1000 needing over 1600 iterations - ** -1000 yielding a large positive number - ** 2000 not converging after 2570 iterations The fix involves scaling down x to [0.5, 1) and expanding the Taylor series with increased precision. The iteration count is now <= 17 with up to 47 multiplications to scale the result in the worst case.
- Optimize bounds check on input value by only checking sign & exponents - Transform negative arguments using exp(x) = 1/exp(-x) - Dynamically set target exponent for argument reduction - Dynamically Set extra precision - Add comments documenting the above points - Optimize inner loop with one less multiplication - Optimize inner loop and result squaring by managing temp values manually - Panic on infinite result
Update with f460a05. Sorry about the substantial changes, but this is however much cleaner and addresses all the points you've raised. |
// term < 1 ulp of sum ⇒ term < 0.5 × 2^(sum.exp-sum.prec+1) | ||
// 0 ≤ term < 1 × 2^term.exp ⇒ 2^term.exp ≤ 2^(sum.exp-sum.prec) | ||
// Because of argument reduction, 1 ≤ sum < 1+2^(-k+1) ⇒ sum.exp == 1 | ||
if term.Sign() == 0 || term.MantExp(nil) <= sum.MantExp(nil) /* ==1 */ -int(sum.Prec()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although sum.MantExp(nil) is known to be 1, I've left it just in case I messed up somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only trivia remain. It's looking very good.
and fix typos.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking very good.
Just a quick note regarding the exit condition of the loop where we check that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks very much for this, lovely improvement.
Fix various issues in exponential() like:
The fix involves scaling down x to [0.5, 1) and expanding the Taylor series with increased precision.
The iteration count is now <= 17 with up to 47 multiplications to scale the result in the worst case.
The results are almost always properly rounded, and off by 1 ulp in the worst case.
I'd like to add a test for this, but because of #225, there is no really clean way to do it.