bug#10781. Remove Float range and Double range [ci: last-only] #6468

eed3si9n · 2018-03-25T15:51:07Z

At the basis of range is an assumption that summation works reliably. Unfortunately it is not true for IEEE floating point number where numbers are stored as approximate fraction of binary numbers. Natually, Double and Float ranges are completely broken. See numbers of issues such as scala/bug#8518, scala/bug#9875, scala/bug#9874, scala/bug#8620, scala/bug#8670, and scala/bug#10759.

I've attempted to fix the Double range in scala/collection-strawman#489 by faking it further using BigDecimal, but ultimately I was not able to overcome the fact that IEEE floats are not suited for ranges.

This removes both Float and Double ranges, as well as the fake Integral[T] instances that underpin their existence.

NthPortal · 2018-04-02T16:00:30Z

src/library/scala/math/Numeric.scala

  }

-  // For Double and BigDecimal we offer implicit Fractional objects, but also one
+  // For BigDecimal we offer implicit Fractional objects, but also one


Should probably be singular "an implicit Fractional object" (admittedly, it wasn't grammatically correct in the first place)

NthPortal · 2018-04-02T16:02:56Z

src/library/scala/math/Numeric.scala

    // logic in Numeric base trait mishandles abs(-0.0)
    override def abs(x: Double): Double = math.abs(x)
  }
  trait DoubleIsFractional extends DoubleIsConflicted with Fractional[Double] {


If DoubleAsIfIntegral is being removed, DoubleIsConflicted can probably be merged into DoubleIsFractional, as it's no longer conflicted (same for FloatIsFractional/FloatIsConflicted)

Sounds good 👍

szeiger · 2018-04-03T16:03:42Z

This should target the 2.13.x-new-collections branch

lrytz · 2018-04-04T10:13:13Z

Yes, but let's just wait until the new collections are in 2.13.x - we only merge to 2.13.x-new-collections what's required to get the collections in 2.13.x.

eed3si9n · 2018-04-05T00:22:28Z

Rebased on top of 2.13.x-new-collections branch for testing, and addressed review comments.

NthPortal · 2018-04-05T23:31:48Z

I'm not disagreeing with @lrytz, but I think the target branch should be 2.13.x-new-collections until that branch is merged, so that we can reason about what this PR actually changes (it doesn't change 30k lines)

dwijnand · 2018-04-06T08:41:37Z

here's the subset diff: 2.13.x-new-collections...eed3si9n:wip/float-range

eed3si9n · 2018-04-12T16:15:57Z

Rebased and squashed.

SethTisue · 2018-04-17T09:58:23Z

why remove immediately, instead of deprecate-in-2.13, remove-in-2.14?

eed3si9n · 2018-04-22T01:49:23Z

why remove immediately, instead of deprecate-in-2.13, remove-in-2.14?

Deprecation is a useful tool to spread the cost of code migration targeting code base that was once in vogue, but no longer considered good, like cars without seatbelts and smoking cigarette in bars. It takes a while for all cars to catch up to the safety standards. That would be our procedure syntax and view bounds.

Floating point ranges, on the other hand, are as broken as cell phones that can catch fire on their own, or car wheels coming off on highways. If someone is (somehow) using it it in 2.12, we should advise them to stop too.

To quote @Ichoran in scala/collection-strawman#489 (comment):

Is it possible to drop numeric ranges that pretend to be for Float and Double? These are just bad news due to numeric imprecision. If you say something like 0.1 to 0.7 by (0.1 + 0.2) you would think you would have three steps, but you either won't, or you will but other code will be randomly inaccurate (because you rounded things enough to make the above work).
(snip)

Normally I don't like breaking changes, but in the case where there is no way to do the right thing except by not actually using the type that is stated, I think the better thing to do is remove the feature.

lrytz · 2018-04-23T08:16:31Z

If we remove this in 2.13.x (which is fine with me), let's deprecate it ASAP in 2.12 (we can still make it for 2.12.6).

Ref scala/bug#10781 This is in prepration of Float range and Double range removal in 2.13.x (scala#6468).

Ref scala/bug#10781 This is in preparation for Float range and Double range removal in 2.13.x (scala#6468).

eed3si9n · 2018-04-23T11:59:59Z

@lrytz Here's a PR for deprecation in 2.12.x - #6550

SethTisue · 2018-04-25T13:49:19Z

I had previously expressed misgivings about removal at scala/collection-strawman#489 (comment), but I think I'm happy to go along with the near-consensus that has developed on it.

SethTisue · 2018-04-26T18:48:31Z

the deprecation PR is merged for 2.12.6.

this PR needs a rebase.

eed3si9n · 2018-04-26T18:51:04Z

@SethTisue Done.

dwijnand · 2018-05-03T07:59:33Z

The deprecation PR merged in 2.12.x was merged into 2.13.x (#6575), meaning this PR needs another rebase. Thank you for playing Code Branches (tm) with scala/scala.

eed3si9n · 2018-05-04T01:52:55Z

Rebased on top of 2.13.x.

Fixes scala/bug#10781 At the basis of range is an assumption that summation works reliably. Unfortunately it is not true for IEEE floating point number where numbers are stored as approximate fraction of binary numbers. Natually, Double and Float ranges are completely broken. See numbers of issues such as scala/bug#8518, scala/bug#9875, scala/bug#9874, scala/bug#8620, scala/bug#8670, and scala/bug#10759. I've attempted to fix the Double range in scala/collection-strawman#489 by faking it further using BigDecimal, but ultimately I was not able to overcome the fact that IEEE floats are not suited for ranges. This removes both Float and Double ranges, as well as the fake `Integral[T]` instances that underpin their existence.

NthPortal · 2018-05-21T22:24:20Z

should we merge this now?

lrytz · 2018-05-24T07:48:47Z

Thanks @eed3si9n, @NthPortal!

Ichoran · 2018-06-14T22:10:38Z

@eed3si9n @NthPortal @SethTisue @som-snytt - What if we change the API?

The whole problem is that we can't hit the endpoints accurately by specifying a step. So what if we abandon the idea of specifying a precise step?

Suppose that 0.1 to 0.7 into 7 will give seven elements that are roughly 0.1, 0.2, ..., 0.7, which is about as much as you can expect from Double anyway. Now there isn't any problem. We don't even have to let until work any more. We could also introduce 0.1 to 0.7 every 0.1 with the meaning that 0.1 and 0.7 are guaranteed to be the endpoints, and it will equally space the points in between as close to 0.1 as it can. Again, no problems with randomly wrong endpoints, because you're required to specify the endpoints. And the intermediates will be as close as anything ever is with Double arithmetic.

WDYT?

Ichoran · 2018-06-14T22:11:59Z

Bikeshed: maybe spaced is better than every for indicating that the spacing is a rough guideline.

som-snytt · 2018-06-14T22:37:18Z

On #6803 I was going to give a shot at something like just ensuring that the mapRange is convenient to use. That is, I don't see the use case as specifying a double step, but just to make it hard to do the wrong thing with range.double with bigdecimal step. I noticed that BigDecimal(.1 * 3) is still an open door. I was going to ask on the forum whether it's been considered to let .1 * 3 : BigDecimal do the obvious. Someone asked about it on the other thread. Also, the to and by could require BigDecimal.

Your idea of doing something mathy with a divided interval is also interesting as an extra credit question.

The dice operator, |/|. As in slice and dicer from Ronco.

eed3si9n · 2018-06-14T23:12:10Z

If we really want some Double operations, I'd say we look into @non's Spire. I remember Intervals talk from nescala a while back - http://plastic-idolatry.com/erik/nescala2015.pdf

NthPortal · 2018-06-16T17:28:07Z

Honestly, I'm not sure that such an API would be that useful, and would still expect confusion

unused since scala#6468

eed3si9n requested a review from Ichoran March 25, 2018 15:51

scala-jenkins added this to the 2.13.0-M4 milestone Mar 25, 2018

eed3si9n changed the title ~~Remove Float range and Double range~~ bug#10781. Remove Float range and Double range Mar 25, 2018

eed3si9n mentioned this pull request Mar 25, 2018

Remove Float range and Double range scala/bug#10781

Closed

eed3si9n changed the title ~~bug#10781. Remove Float range and Double range~~ bug#10781. Remove Float range and Double range [ci: last-only] Mar 26, 2018

eed3si9n force-pushed the wip/float-range branch from 75eecc6 to c883a7f Compare March 31, 2018 14:08

NthPortal mentioned this pull request Apr 2, 2018

bug#10511 Add total orderings for Float and Double #6410

Merged

NthPortal suggested changes Apr 2, 2018

View reviewed changes

eed3si9n force-pushed the wip/float-range branch from c883a7f to 45cee0e Compare April 5, 2018 00:18

eed3si9n force-pushed the wip/float-range branch 3 times, most recently from 485b2b2 to e41fae8 Compare April 12, 2018 04:06

NthPortal approved these changes Apr 21, 2018

View reviewed changes

eed3si9n added a commit to eed3si9n/scala that referenced this pull request Apr 23, 2018

Deprecate Float range and Double range

4d509e6

Ref scala/bug#10781 This is in prepration of Float range and Double range removal in 2.13.x (scala#6468).

eed3si9n added a commit to eed3si9n/scala that referenced this pull request Apr 23, 2018

Deprecate Float range and Double range

340b899

Ref scala/bug#10781 This is in preparation for Float range and Double range removal in 2.13.x (scala#6468).

eed3si9n mentioned this pull request Apr 23, 2018

Deprecate floating-point Ranges #6550

Merged

NthPortal mentioned this pull request Apr 25, 2018

Forward more methods for reverse orderings #6360

Merged

eed3si9n force-pushed the wip/float-range branch from e41fae8 to feca33c Compare April 26, 2018 18:50

SethTisue modified the milestones: 2.13.0-M4, 2.13.0-M5 Apr 26, 2018

eed3si9n force-pushed the wip/float-range branch from feca33c to 7db5bcc Compare May 4, 2018 01:51

eed3si9n force-pushed the wip/float-range branch from 7db5bcc to 16f58a1 Compare May 4, 2018 06:39

lrytz approved these changes May 24, 2018

View reviewed changes

lrytz merged commit 0f70451 into scala:2.13.x May 24, 2018

eed3si9n deleted the wip/float-range branch May 24, 2018 15:16

xuwei-k added a commit to xuwei-k/scala-1 that referenced this pull request Sep 1, 2018

remove NumericRange#mapRange

aa0fd0f

unused since scala#6468

xuwei-k mentioned this pull request Sep 1, 2018

remove NumericRange#mapRange #7161

Merged

xuwei-k mentioned this pull request Sep 14, 2018

Null Pointer Exception in Range.Double.toArray scala/bug#8782

Closed

xuwei-k mentioned this pull request Nov 30, 2022

Added wart for floating point numeric ranges wartremover/wartremover#209

Closed

bug#10781. Remove Float range and Double range [ci: last-only] #6468

bug#10781. Remove Float range and Double range [ci: last-only] #6468

Uh oh!

Conversation

eed3si9n commented Mar 25, 2018

Uh oh!

NthPortal Apr 2, 2018

Choose a reason for hiding this comment

Uh oh!

NthPortal Apr 2, 2018

Choose a reason for hiding this comment

Uh oh!

eed3si9n Apr 2, 2018

Choose a reason for hiding this comment

Uh oh!

szeiger commented Apr 3, 2018

Uh oh!

lrytz commented Apr 4, 2018

Uh oh!

eed3si9n commented Apr 5, 2018

Uh oh!

NthPortal commented Apr 5, 2018

Uh oh!

dwijnand commented Apr 6, 2018

Uh oh!

eed3si9n commented Apr 12, 2018

Uh oh!

SethTisue commented Apr 17, 2018

Uh oh!

eed3si9n commented Apr 22, 2018

Uh oh!

lrytz commented Apr 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eed3si9n commented Apr 23, 2018

Uh oh!

SethTisue commented Apr 25, 2018

Uh oh!

SethTisue commented Apr 26, 2018

Uh oh!

eed3si9n commented Apr 26, 2018

Uh oh!

dwijnand commented May 3, 2018

Uh oh!

eed3si9n commented May 4, 2018

Uh oh!

NthPortal commented May 21, 2018

Uh oh!

lrytz commented May 24, 2018

Uh oh!

Ichoran commented Jun 14, 2018

Uh oh!

Ichoran commented Jun 14, 2018

Uh oh!

som-snytt commented Jun 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eed3si9n commented Jun 14, 2018

Uh oh!

NthPortal commented Jun 16, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

lrytz commented Apr 23, 2018 •

edited

Loading

som-snytt commented Jun 14, 2018 •

edited

Loading