Bug #18018
closedFloat#floor / truncate sometimes result that is too small.
Description
291.4.floor(1) # => 291.4 (ok)
291.4.floor(2) # => 291.39 (not ok)
291.4.floor(3) # => 291.4 (ok)
291.4.floor(4) # => 291.4 (ok)
291.4.floor(5) # => 291.39999 (not ok)
291.4.floor(6) # => 291.4 (ok)
g = f.floor(n)
, for n > 0
must return the highest float that has the correct properties:
-
g
<=f
-
g
's decimal string representation has at mostn
digits
I'll note that floor
should be stable, i.e. f.floor(n).floor(n) == f.floor(n)
for all f
and n
.
Same idea for truncate
, except for negative numbers (where (-f).truncate(n) == -(f.floor(n))
for positive f
).
Noticed by Eustáquio Rangel but posted on the mailing list.
Please do not reply that I need to learn how floats work. Note that example given in doc (0.3/0.1).floor == 2
is not this issue, since 0.3/0.1 #=> 2.9999999999999996
Updated by sawa (Tsuyoshi Sawada) over 3 years ago
With:
g's decimal string representation has at most n digits
I think you mean:
g's decimal string representation has at most n decimal (or fractional) digits
(By the way, note here that the word "decimal" is used twice under different meanings: 1. base ten, and 2. the fractional part)
Updated by marcandre (Marc-Andre Lafortune) over 3 years ago
sawa (Tsuyoshi Sawada) wrote in #note-1:
With:
g's decimal string representation has at most n digits
I think you mean:
g's decimal string representation has at most n decimal (or fractional) digits
(By the way, note here that the word "decimal" is used twice under different meanings: 1. base ten, and 2. the fractional part)
Yes, that is what I meant, thanks.
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
- Status changed from Open to Feedback
The algorithm for Float#floor
with the ndigits
argument is basically (omitting the overflow/underflow handling):
class Float
def floor_ndigits(ndigits)
x = (10**ndigits).to_f
(self * x).floor / x
end
end
Let's see what self * x
is in each case:
291.4 * 10 # 2914.0
291.4 * 100 # 29139.999999999996
291.4 * 1000 # 291400.0
291.4 * 10000 # 2914000.0
291.4 * 100000 # 29139999.999999996
291.4 * 1000000 # 291400000.0
This issue also goes the other direction:
f = 291.39999999999997
6.times.map{|i| f.floor(i)}
# => [291, 291.4, 291.39, 291.4, 291.4, 291.39999]
Float#floor
results are inexact, because Float
itself is inexact, so I don't think the current behavior is a bug.
marcandre (Marc-Andre Lafortune) wrote:
g = f.floor(n)
, forn > 0
must return the highest float that has the correct properties:
g
<=f
g
's decimal string representation has at mostn
digits
I think these are both true in these cases. 291.4, 291.39, and 219.39999 are all <= 291.4, and the decimal string representation has at most the number of digits specified after the decimal point.
I'll note that
floor
should be stable, i.e.f.floor(n).floor(n) == f.floor(n)
for allf
andn
.
This is also true:
291.4.floor(2).floor(2) == 291.4.floor(2) # true
291.4.floor(5).floor(5) == 291.4.floor(5) # true
@marcandre (Marc-Andre Lafortune) If you still think this is a bug, could you explain why, and ideally the algorithm that should be used instead?
Updated by marcandre (Marc-Andre Lafortune) over 3 years ago
jeremyevans0 (Jeremy Evans) wrote in #note-3:
This issue also goes the other direction:
f = 291.39999999999997
Well, that f
is the same float (291.4), I don't understand your point, sorry.
marcandre (Marc-Andre Lafortune) wrote:
g = f.floor(n)
, forn > 0
must return the highest float that has the correct properties:
g
<=f
g
's decimal string representation has at mostn
digitsI think these are both true in these cases. 291.4, 291.39, and 219.39999 are all <= 291.4, and the decimal string representation has at most the number of digits specified after the decimal point.
Maybe you missed "the highest float" in my definition? 291.4 is the only float that fits the definition.
@marcandre (Marc-Andre Lafortune) If you still think this is a bug, could you explain why, and ideally the algorithm that should be used instead?
I still think it is a bug.
A correct algorithm seem to be to rely on Rational#floor
:
class Float
def correct_floor(n)
Rational(self).floor(n).to_f
end
end
f = 291.4
p 6.times.map{|i| f.correct_floor(i)}
# => [291.0, 291.4, 291.4, 291.4, 291.4, 291.4]
Updated by jeremyevans0 (Jeremy Evans) over 3 years ago
- Status changed from Feedback to Open
marcandre (Marc-Andre Lafortune) wrote in #note-4:
marcandre (Marc-Andre Lafortune) wrote:
g = f.floor(n)
, forn > 0
must return the highest float that has the correct properties:
g
<=f
g
's decimal string representation has at mostn
digitsI think these are both true in these cases. 291.4, 291.39, and 219.39999 are all <= 291.4, and the decimal string representation has at most the number of digits specified after the decimal point.
Maybe you missed "the highest float" in my definition? 291.4 is the only float that fits the definition.
I did miss that :). I agree with you that it is reasonable definition.
A correct algorithm seem to be to rely on
Rational#floor
:class Float def correct_floor(n) Rational(self).floor(n).to_f end end f = 291.4 p 6.times.map{|i| f.correct_floor(i)} # => [291.0, 291.4, 291.4, 291.4, 291.4, 291.4]
Which platform are you running on? All versions of Ruby I tried on OpenBSD/amd64 and Windows x64 gave the following output for this code:
[291.0, 291.3, 291.39, 291.399, 291.3999, 291.39999]
I think a simpler solution is to increment by 1 before dividing. If that is too big, then use the previous calculation. I submitted a pull request for that: https://github.com/ruby/ruby/pull/4681
Updated by marcandre (Marc-Andre Lafortune) over 3 years ago
jeremyevans0 (Jeremy Evans) wrote in #note-5:
A correct algorithm seem to be to rely on
Rational#floor
:class Float def correct_floor(n) Rational(self).floor(n).to_f end end f = 291.4 p 6.times.map{|i| f.correct_floor(i)} # => [291.0, 291.4, 291.4, 291.4, 291.4, 291.4]
Which platform are you running on? All versions of Ruby I tried on OpenBSD/amd64 and Windows x64 gave the following output for this code:
[291.0, 291.3, 291.39, 291.399, 291.3999, 291.39999]
My bad 🤦♂️ Rational#floor makes sense for rationals, but is not what we can use. Not sure how I got confused.
I think a simpler solution is to increment by 1 before dividing. If that is too big, then use the previous calculation. I submitted a pull request for that: https://github.com/ruby/ruby/pull/4681
Sounds reasonable
Updated by jeremyevans (Jeremy Evans) over 3 years ago
- Status changed from Open to Closed
Applied in changeset git|35e467080ca35a9a129e95f802f102c3bc0a81b3.
Make Float#floor with ndigits argument handle error
The previous implementation could result in a returned
float that is 1/(10**ndigits) too low. First try adding
one before dividing, and if that results in a value that is
greater than the initial number, then try the original
calculation.
Spec added for ciel, but the issue doesn't appear to affect
ciel, at least not for the same number. If the issue does
effect ciel, a similar fix could probably work for it.
Fixes [Bug #18018]