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
Range.BigDecimal and Range.Double misses last element #4985
Comments
Imported From: https://issues.scala-lang.org/browse/SI-4985?orig=1
|
@paulp said: |
Thomas Switzer (tixxit) said: 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. |
Thomas Switzer (tixxit) said: |
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.
The text was updated successfully, but these errors were encountered: