Skip to content
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

Float: more accurate to_s #2650

Merged
merged 1 commit into from
May 26, 2016
Merged

Float: more accurate to_s #2650

merged 1 commit into from
May 26, 2016

Conversation

asterite
Copy link
Member

This PR makes Float64#to_s produce a more correct output.

This targets two issues: #2220 and #2643

The "correct" way to do it would be to use Dragon4 or some other well-known algorithm, as commented here. However, the algorithms are full of math that I don't want to learn right now, and code that is always implemented in C with a lot of lines of code full of defines and low-level hacks. We can probably do it, but later.

For now, I use snprintf with "%.17g", which produces a very good result but sometimes ends up with runs of zeros or nines. I check if there are such runs near the end of the string and remove them (case of zero) or replace them (case of nine, add one and carry). This makes Float64#to_s twice as slow, but still faster than Ruby:

time = Time.now
a = 0
float = ARGV[0].to_f
1_000_000.times do
  a += float.to_s.bytesize
end
puts Time.now - time

Ruby takes 0.95s, Crystal takes 0.77s with an argument "0.6337845278325672". It used to take 0.31s before this change. But, I don't think this is going to be a program's bottleneck, and for now I prefer a more accurate result than a faster but wrong one. I only compare it to Ruby because if it's slow but people use it, and in this case it's slightly faster, then it's still good (or "not that bad").

For Float32, the old algorithm is used (use snprintfwith "%.g", first converting to Float64) because it seems snprintf doesn't handle floats. And Float32 is less used, so we can care of that later.

An example of this change:

puts Time.now.epoch_f

# Before: 1.46422e+09
# After: 1464218231.9172699

Verified

This commit was signed with the committer’s verified signature.
snyk-bot Snyk bot
@asterite asterite force-pushed the feature/float_to_s branch from 6ebb368 to 649c990 Compare May 25, 2016 23:56
@asterite asterite merged commit 0cb2388 into release/0.17 May 26, 2016
@asterite asterite deleted the feature/float_to_s branch May 26, 2016 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant