Range.BigDecimal and Range.Double misses last element

[scabug]

This is fairly simple to reproduce:

Range.Double(0.0, 1.0, 0.3)

Gives NumericRange(0.0, 0.3, 0.6). It should give NumericRange(0.0, 0.3, 0.6, 0.9).

It misses this last element because, for some reason, when calculating the length (in NumericRange.count), the "remainder" (num.rem(diff, step)) is converted to a Long (num.toLong(num.rem(diff, step))) and then compares it to num.zero. Any "remainder" < 1.0 would mean the last element is omitted, since num.toLong would force it to 0, which then compares with num.zero as true. In the above case, the remainder is 0.1, so the problem presents itself.

I've attached a simple patch which removes the num.toLong conversion and fixes the issue.

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-4985?orig=1 Reporter: Thomas Switzer (tixxit) Affected Versions: 2.9.0, 2.9.1, 2.9.2 Attachments:

• numericrange-count.patch (created on Sep 9, 2011 4:38:11 PM UTC, 619 bytes)
• numeric-range-remainder.patch (created on Oct 30, 2011 1:40:56 AM UTC, 950 bytes)

[scabug]

@paulp said: Have you tested that patch against existing tests? This is a messed up situation right here: the count code is written assuming it's dealing with an integral type, which seems pretty reasonable given that the implicit parameter is Integral, not Numeric. DoubleAsIfIntegral was an ill-conceived idea, I would definitely like to axe that. Anyway, that's why it converts to Long, and it does so in an attempt to recognize overflow. On the presumption there are test cases for the overflow check, you have probably broken those.

[scabug]

Thomas Switzer (tixxit) said: Hi Paul, all the tests pass, except for fft.scala, because it uses LESS boxed longs with my patch (I'll attach the patch for fft.check + the patch above so that it passes).

I don't think overflow is an issue (but I may also have misunderstand what you meant). The problem is from the remainder being converted to a Long, not the actual count (which makes sense as a Long). Since the remainder, after being converted to Long, is being compared against num.zero (not 0L), then you are doing (eg.) Double -> Long -> Double so that it can be compared against num.zero. This is also why there are less boxed longs in the FFT test after applying this patch, because w/o it we are needlessly unboxing then reboxing longs so we can compare them to num.zero (num is an Integral[Long] in fft.scala).

The remainder check is only there to handle the case where the right side is excluded and the right side (the end) would have been included had it been inclusive. Converting it to a Long first before comparing it w/ num.zero doesn't have any benefits (and also leads to this bug) and the actual "remainder" variable is only ever used in its comparison with num.zero.

[scabug]

Thomas Switzer (tixxit) said: Fix for the bug + fix for the failed fft check.

[scabug]

Commit Message Bot (anonymous) said: (extempore in r25918) Fix for NumericRange boundary condition.

Contributed by Thomas Switzer. Closes #4985, no review.