-
Notifications
You must be signed in to change notification settings - Fork 79
Rewrite BigDecimal#sqrt in ruby with improved Newton's method #381
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
base: master
Are you sure you want to change the base?
Conversation
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.
@tompng I like this approach. Please check a comment I left.
lib/bigdecimal.rb
Outdated
if infinite? == 1 | ||
exception_mode = BigDecimal.mode(BigDecimal::EXCEPTION_ALL) | ||
raise FloatDomainError, "Computation results in 'Infinity'" if exception_mode.anybits?(BigDecimal::EXCEPTION_INFINITY) |
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.
@tompng, how do you think about extracting the infinity check and exception-raising part as a method?
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.
This is sqrt-specific condition. For example, Math.atan(infinity)=π/2
, `"Computation results in 'Infinity'" is not a correct message.
We can re-consider adding a common infinity check method later, for unbounded monotonic increasing function and other types, but I think it's too early for now.
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.
Sorry I misread the comment. Changed to:
return BigMath._infinity_computation_result if infinite? == 1
This method is added in #389 and used in BigDecimal#power
, BigMath.exp
and BigMath.log
.
prec = [prec, n_digits].max | ||
|
||
ex = exponent / 2 | ||
x = self * BigDecimal("1e#{-ex * 2}") |
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.
@tompng, I think this square root calculation can be faster if we use decimal shift operations here and on line 37, as we previously discussed. What about this?
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.
I think it's good to use decimal shift operator. I've updated #324 as ready for review.
f045650
to
db8554d
Compare
db8554d
to
27d554a
Compare
27d554a
to
a8976b6
Compare
Implement sqrt in ruby with faster Newton's method implementation.
Related to #323 (doing the same rewrite with Integer.sqrt)
Example calculating BigDecimal(2).sqrt in 116 precision
Minimum loop count, minimum precision in each step.
The original c implementation was slow for large precision because it was not doing this.
Small precision: gets slower
Large precision: faster