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

Double.Range misses last element #9348

Closed
scabug opened this issue Jun 5, 2015 · 4 comments
Milestone

Comments

@scabug
Copy link

@scabug scabug commented Jun 5, 2015

Double.Range produces wrong results, leaving out the last element whenever (end-start) == ((end-start) / step) * step in BigDecimals. Just browsing through the sources, I can't find any reason why BigDecimalAsIfIntegral.quot, DoubleAsIfIntegral.quot and FloatAsIfIntegral.quot are implemented with regular division operator / instead of BigDecimal.quot.

Bug example:

Double.Range(0, 1, 0.7)
res1: NumericRange(0.0)
Double.Range(0, 0.6, 0.25)
res2: NumericRange(0.0, 0.25)

There is a related issue from 2011 at #4985

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 5, 2015

Imported From: https://issues.scala-lang.org/browse/SI-9348?orig=1
Reporter: @vuakko
Assignee: @vuakko
Affected Versions: 2.11.6

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 16, 2015

@axel22 said:
Whoops! Looks like a regression!

Welcome to Scala version 2.10.4 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_21).
Type in expressions to have them evaluated.
Type :help for more information.

scala> Range.Double(0, 1, 0.7)
res0: scala.collection.immutable.NumericRange[Double] = NumericRange(0.0, 0.7)
Welcome to Scala version 2.11.1 (Java HotSpot(TM) 64-Bit Server VM, Java 1.7.0_21).
Type in expressions to have them evaluated.
Type :help for more information.

scala> Range.Double(0.0, 1.0, 0.7)
res0: scala.collection.immutable.NumericRange[Double] = NumericRange(0.0)
@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 17, 2015

@vuakko said:
The regression wasn't caught, because there are currently no "real" tests for floating point ranges where
either start, end or step would be different from a whole integer. This is a problem especially since the current
NumericRange.count method has fully separate code for "effectively integer" ranges and so actually most of the logic
in count seems to have no tests.

Suggestion: I will add a bunch more tests for possible NumericRange pitfalls.

2.10.x seems to do num.toLong(num.quot(...)) which avoids the problem, but that was generalized away in 994de8ff.

@scabug

This comment has been minimized.

Copy link
Author

@scabug scabug commented Jun 17, 2015

@scabug scabug closed this Jun 17, 2015
@scabug scabug added this to the 2.11.7 milestone Apr 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.