Un-deprecate default floating point Orderings

[NthPortal]

Fixes scala/bug#10511. One of 3 proposed solutions; the others are #8613 and #8614.

[sjrd]

This is the only alternative that is acceptable in a minor version, IMO. The other two PRs intentionally break valid existing code.

[NthPortal]

#8614 (comment)

[eed3si9n]

@NthPortal As @sjrd suggested I've opened a new issue scala/bug#11844. Could you update the commit message to point to it please?

[Ichoran]

@sjrd - Actually, this is the PR that intentionally and silently breaks valid existing code, because in 2.12 (and earlier), the behavior was that of IeeeOrdering, but this switches it to TotalOrdering.

[eed3si9n]

@Ichoran Scala 2.12.10 returned the following for min and max:

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).max
res1: Double = 3.0

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).min
res2: Double = 0.0

This behavior does not match either the java.util.Collections sorting order or IEEE, since IEEE would return NaN for both.

[eed3si9n]

For List#min and List#max returning the first and last element of the sort makes sense and it's consistent with java.util.Collections.max.

[Ichoran]

@eed3si9n - 2.12.0's min and max aren't implemented correctly for a non-total-order. I don't advocate restoring that behavior, as it's unpredictable. It effectively deletes the list up to the last NaN (including it, if the list goes beyond).

[eed3si9n]

@Ichoran Is there a sequence that would make Scala 2.13.1 (TotalOrdering) sort differently from Scala 2.12 or IEEEOrdering? Using Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0) they all seem to sort as Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN) - scala/bug#11844 (comment)

Assuming that's the case, the difference between Total vs IEEE comes down to the result of List#min and List#max. Since we both agree that Scala 2.12's results were broken, I don't think picking either ordering would affect the compatibility with Scala 2.12.

Continuing to go Total by default and returning the last value of the List#sorted as List#max seems most intuitive and is also consistent with java.util.Collections.max, which is different from java.lang.Math.max.

[Ichoran]

@eed3si9n - Both 2.13.0 and 2.12.whatever use the Java ordering, so the sorting should be identical between the two (at least in the absence of bugs). Indeed, both IeeeOrdering and TotalOrdering produce the exact same ordering!

The question here is not about sort order. It is also not entirely about the collections max method. The issue is that you have abstracted ordering of numeric operations with Ordering, and it's really weird if the answers come out differently when you abstract them. You at least need a warning that a max b isn't the same as a max b in a generic context. We changed the behavior of a max b in 2.13 because some people disliked (and quite reasonably so) that max, <, and compare didn't agree with each other. But that is a really dangerous thing to do silently, hence the deprecation message.

Again, this is only about max/min (and to a lesser extent < and friends). When you sort things, everyone agrees on the same ordering (well, except we don't agree with IEEE754, but that's reasonable given that we don't have any visibility into different kinds of NaN). It's just that we were using big clumsy tools to fight the min/max problem and sorted was a civilian casualty.

[eed3si9n]

On Scala 2.13.1, 2.0 min NaN continues to return NaN. Is there another a max b?

[Ichoran]

@eed3si9n - Yes, the

def example[A: Ordering](a: A, aa: A): A = {
  val ord = implicitly[Ordering[A]]
  import ord._
  a min aa
}

one you have to use in a generic context.

[eed3si9n]

ok. That seems to be an example where 2.12.x and 2.13.1 do give different results.

2.12.10

scala> import java.lang.Double.NaN
import java.lang.Double.NaN

scala> def typeclassMin[A: Ordering](a: A, aa: A): A = {
     |   val ord = implicitly[Ordering[A]]
     |   import ord._
     |   a min aa
     | }
typeclassMin: [A](a: A, aa: A)(implicit evidence$1: Ordering[A])A

scala> typeclassMin(2.0, NaN)
res3: Double = NaN

2.13.1

scala> typeclassMin(2.0, NaN)
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res1: Double = 2.0

scala> { import Ordering.Double.IeeeOrdering; typeclassMin(2.0, NaN) }
res3: Double = NaN

scala> { import Ordering.Double.TotalOrdering; typeclassMin(2.0, NaN) }
res4: Double = 2.0

[NthPortal]

That seems to be an example where 2.12.x and 2.13.1 do give different results.

mhm. the default Ordering[Double] is a total ordering in 2.13, but not in 2.12

[eed3si9n]

Given the discussion on scala/bug#11844, I'm no longer sure if this is the right way to go - scala/bug#11844 (comment)

No longer sure

[NthPortal]

I continue to lean towards Sébastien's proposed solution (scala/bug#11844 (comment))

[SethTisue]

Reopened, as I believe the leading contender currently is a version of this PR that replaces the deprecation warning with an -Xmigration warning (which we will prominently release-note for 2.13.2, along with going back and retroactively release-noting it in 2.13.0, too).

[SethTisue]

@NthPortal are you interested in adding the migration warning? in the next week or so? If you're too busy or just not interested, it's fine, one of us from the core team can do it

[NthPortal]

@SethTisue If I had a good idea of what the wording should be, I'd be happy to do it; however, I don't have the capacity right now to come up with a good one.

If someone provides one, I'm happy to adjust this PR to use it (and un-delete the partest; possibly rename it?). Or someone else could adjust this PR, or open a new PR.

Thanks for keeping this rolling

[NthPortal]

closing in favour of #8721