Bug #18018: Float#floor / truncate sometimes result that is too small.

• Status: Closed
• Assignee: -
• Target version: 3.1
• Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN

[marcandre (Marc-Andre Lafortune)]

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 most n 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

[sawa (Tsuyoshi Sawada)]

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)

[marcandre (Marc-Andre Lafortune)]

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.

[jeremyevans0 (Jeremy Evans)]

• 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), for n > 0 must return the highest float that has the correct properties:

• g <= f
• g's decimal string representation has at most n 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 all f and n.

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?

[marcandre (Marc-Andre Lafortune)]

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), for n > 0 must return the highest float that has the correct properties:

• g <= f
• g's decimal string representation has at most n 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.

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]

[jeremyevans0 (Jeremy Evans)]

• Status changed from Feedback to Open

marcandre (Marc-Andre Lafortune) wrote in #note-4:

marcandre (Marc-Andre Lafortune) wrote:

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 most n 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.

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

[marcandre (Marc-Andre Lafortune)]

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

• 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]