List(1.0, -1.0).sorted gives a deprecation warning #11844

eed3si9n · 2020-01-02T15:50:12Z

Originally posted by @odersky in #10511 (comment)

Ref scala/scala#6323
Ref scala/scala#6410

steps

scala> List(1.0, -1.0).sorted
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res0: List[Double] = List(-1.0, 1.0)

problem

It gives a deprecation warning. I think this is very bad, not to say catastrophic.

expectation

These things should just work. I don't care what ordering is used and whether it is inconsistent or not (we all know IEEE is broken and unfixable). But we should do the right thing for everyday users. As an everyday user I do not care what the ordering does for NaN and -0.0, maybe I do not even know that these weird numbers exist! (and they should not exist, if I go by common mathematical logic). But I do care that I can sort a list of doubles without going into the deeper mysteries of Scala's implicit system. This requirement is now violated.

notes

I think we should revert ASAP. We should keep the old total ordering and offer IEEE as an optional alternative that needs to be imported explicitly.

eed3si9n · 2020-01-02T15:53:59Z

@NthPortal wrote:

I think we should revert ASAP. We should keep the old total ordering and offer IEEE as an optional alternative that needs to be imported explicitly.

The issue is, the old behaviour was the IEEE ordering, so if we switch to a total ordering by default now, there's no transition period. Are we okay with that? (see also scala/scala#6410 (comment))

eed3si9n · 2020-01-02T15:54:52Z

@odersky wrote:

I believe we should simply keep the old behavior. My opinion about NaN and -0.0 is:

we are under no obligation to ensure that primitive types and boxed types behave the same
we are under no obligation to make the ordering total or not.
we should do the same thing that Java does
we should not make operations that do not involve NaN and -0.0 harder than before.

My rationale is that these constructs are illogical anyway. No matter what you do, there will be weirdness. So, the best thing to do is not upset the apple cart.

eed3si9n · 2020-01-02T15:55:26Z

@NthPortal wrote:

thoughts @Ichoran @szeiger @SethTisue?

eed3si9n · 2020-01-02T15:56:16Z

@NthPortal wrote:

FWIW, Java uses a total ordering on primitives:

scala> Array[Double](5.0, 3.0, NaN, 4.0)
res0: Array[Double] = Array(5.0, 3.0, NaN, 4.0)

scala> java.util.Arrays.sort(res0)

scala> res0
res3: Array[Double] = Array(3.0, 4.0, 5.0, NaN)

eed3si9n · 2020-01-02T15:57:03Z

@NthPortal wrote:

Personally, I think we have an obligation to have Orderings be internally consistent (compare(_, _) < 0 iff lt(_, _) == true, etc.), which happens to mean having a total ordering.

Another possibility of course is to have the default ordering just throw an exception if you give it NaN ;P

If we must un-deprecate it, I feel the default should be the total ordering. I don't care particularly about how 0.0/-0.0 compare, as long as it's consistent, but I care that operations involving NaN (which is concerningly easy to summon) behave sanely and consistently. It happens to be that the current (deprecated) default is a total ordering, so we can just remove the annotation and fix up a few tests.

eed3si9n · 2020-01-02T15:58:18Z

@odersky wrote:

Java uses total ordering right? So why and when did Scala deviate from this to use IEEE ordering?

eed3si9n · 2020-01-02T15:59:40Z

@NthPortal wrote:

@odersky it changed in #5104 and scala/scala#76

In essence the issue is that there is an expectation (a valid one) that min and max should comply with the IEEE specification (i.e. with java.lang.Math.{min,max}). Unfortunately, that is not consistent with a total ordering.

I highly recommend reading all of the discussions on scala/scala#6323 and scala/scala#6410 (even though they're long) for the full context of how we decided on the solution we chose.

eed3si9n · 2020-01-02T16:00:28Z

@dwijnand wrote:

It's a shame that Java is inconsistent between java.util.Arrays.sort and java.lang.Math.{min,max}.

Can we capture this explanation and put it in on a page of docs.scala-lang.org and refer to it in the Scaladoc? Or put the whole explanation in the Scaladoc. Note @som-snytt had some thoughts around those docs in scala/scala#8191 (comment) too.

eed3si9n · 2020-01-02T16:02:27Z

@odersky wrote:

I have already expressed my opinion. List(1.0, -1.0).sorted just has to work, no excuses allowed.

The fact that it stopped working, despite the fact that a lot of eyes looked at it, is worrying to me. Did anyone think it was acceptable that this would cause an implicit ambiguity message? Or did it just slip through with nobody realizing the consequences?

eed3si9n · 2020-01-02T16:03:08Z

@odersky wrote:

Can we capture this explanation and put it in on a page of docs.scala-lang.org and refer to it in the Scaladoc? Or put the whole explanation in the Scaladoc. Note @som-snytt had some thoughts around those docs in scala/scala#8191 (comment) too.

I would be skeptical about this. The fact is: NaN is irredeemably broken. It was introduced as a hack since computer manufacturers in the 70s did not think they could provide precise exceptions and Fortran did not provide exception handling. But it makes no sense, mathematically. So yes, of course things will end up being inconsistent somewhere, and the only sensible strategy is to accept that and ignore the problem (which is what Java did, and I think they did the right thing).

eed3si9n · 2020-01-02T16:04:31Z

@dwijnand wrote:

Did anyone think it was acceptable that this would cause an implicit ambiguity message? Or did it just slip through with nobody realizing the consequences?

Speaking only for myself, I thought it was acceptable, given the circumstances.

Can we capture this explanation and put it in on a page of docs.scala-lang.org and refer to it in the Scaladoc? Or put the whole explanation in the Scaladoc. Note @som-snytt had some thoughts around those docs in scala/scala#8191 (comment) too.

I would be skeptical about this. The fact is: NaN is irredeemably broken.

I'll give others a chance to weigh in (particularly Nth, as I expect she's spent a long time looking at all the angles), but I still think that whatever happens (change or no change) it would be good to have some kind of documentation capturing the trade-off chosen.

eed3si9n · 2020-01-02T16:05:22Z

@odersky wrote:

I agree it's good to document, but any such documentation cannot be a reason to keep the current broken behavior.

For background: I came across this because a professor of mathematics was stumped by the behavior and did not know how to proceed.

eed3si9n · 2020-01-02T16:17:04Z

There are 3 pull requests for this by @NthPortal:

Ichoran · 2020-01-02T17:43:37Z

The biggest problem here, by far, is that we're abusing deprecations to provide an informational message. If the thing said, always, and only for min/max/gt etc., not compare or sorted

Note: this code assumes that NaN is handled under a "total ordering", where NaN
is considered to be larger than positive infinity in both sorting and min/max.
  If you wish to keep using a total ordering, import scala.math.Ordering.Double.TotalOrdering
  If you prefer to use IEEE754 ordering, import scala.math.Ordering.Double.IeeeOrdering

and on web interfaces, this was reduced to a popup, then there's no problem. But right now, we get a weird cryptic message about deprecations, without showing the deprecation, on an utterly normal-looking piece of code.

On the other hand, which behavior is followed can be critical for the correct operation of numeric code, yet difficult to debug when behavior changes, so I don't think we can just go changing it without giving users some indication that they need to examine the code.

The right solution is to introduce another trait, Extremal, that is used for min and max. Then sorted can use a total ordering, but min and max use the IEEE754 behavior.

This isn't binary compatible, and even then it's inconsistent with 2.13 behavior, but it matches 2.12 behavior and follows the "principle of least surprise" (i.e. things work the same way).

A pragmatic solution might be just to make DeprecatedDoubleOrdering un-deprecated, and restore the internal inconsistency in it where min and max would give the IEEE754 behavior, which isn't consistent with the answer you'd get from compare. Then things would just work, and the danger would be averted, and the afficionados who wanted an internally consistent ordering could go get the one they wanted.

eed3si9n · 2020-01-02T18:33:53Z

So here are some proposed criteria of success:

don't use deprecation on List(1.0, -1.0).sorted or .max
we should do the same thing that Java does
principle of least surprise (don't silently break simple looking code from Scala 2.12?)
bincompat with Scala 2.13.0
internal consistency does not matter

Is there a solution that fit this bill?

sjrd · 2020-01-02T19:50:33Z

Un-deprecate DeprecatedOrdering
Change the implementation of x min y and x max y so that the special-case {Float,Double}.DeprecatedOrdering, inn order to apply IEEE behavior.

Disgusting, but it fits the bill except for the fact that sorting behavior is still not the same as 2.12 (but I believe we all agree that the behavior of sorting in 2.12 was utterly broken, and that the new behavior is better).

Better the solution in a future where we can break bin compat: rewrite x min y so that they don't use 5 layers of abstraction that are slow and don't even work, along with all the other operations in RichXyz (I have a branch with that already).

eed3si9n · 2020-01-02T19:58:19Z

summary

java.util.Collections.max and java.lang.Math.max do different things
- Both java.util.Collections.min / max and Scala 2.13.1 select min and max values to be -0.0 and NaN respectively.
- Scala 2.12.10's Array#min and Array#max values are 0.0 and 3.0 respectively, which seems like an implementation bug.
sorting looks ok!
- java.util.Arrays, java.util.Collections, Scala 2.12.10, and Scala 2.13.1 with or without IEEE ordering all sorts Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0) to be Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN). << @NthPortal is there a corner case that sorts differently?

Edit:

generic min and max operators did in fact change
- java.lang.Math.min / max uses NaN for both. 2.0 max NaN and 2.0 min NaN both return NaN as well for both Scala 2.12.10 and 2.13.10. However, Scala 2.12.10's typeclassMin(2.0, NaN) returns NaN whereas Scala 2.13.1 returns 2.0.

java.util.Arrays sorting primitive double

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

scala> val xs = Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0); java.util.Arrays.sort(xs); xs
xs: Array[Double] = Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)
res0: Array[Double] = Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

java.util.Collections sorting boxed java.lang.Double

scala> val ys = java.util.Arrays.asList(2.0: java.lang.Double, 1.0: java.lang.Double, java.lang.Double.NaN: java.lang.Double, 3.0: java.lang.Double, 0.0: java.lang.Double, -0.0: java.lang.Double); java.util.Collections.sort(ys); ys
ys: java.util.List[Double] = [-0.0, 0.0, 1.0, 2.0, 3.0, NaN]
res4: java.util.List[Double] = [-0.0, 0.0, 1.0, 2.0, 3.0, NaN]

scala> java.util.Collections.max(ys)
res5: Double = NaN

scala> java.util.Collections.min(ys)
res6: Double = -0.0

java.lang.Math

scala> java.lang.Math.max(2.0, NaN)
res8: Double = NaN

scala> java.lang.Math.min(2.0, NaN)
res9: Double = NaN

Scala 2.12.10

[info] Starting scala interpreter...
Welcome to Scala 2.12.10 (OpenJDK 64-Bit Server VM, Java 1.8.0_222).
Type in expressions for evaluation. Or try :help.

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

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
res0: Array[Double] = Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

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

scala> List(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
res3: List[Double] = List(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

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

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

scala> 2.0 max NaN
res6: Double = NaN

scala> 2.0 min NaN
res7: Double = NaN

scala> :paste
// Entering paste mode (ctrl-D to finish)

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

// Exiting paste mode, now interpreting.

typeclassMax: [A](a: A, aa: A)(implicit evidence$1: Ordering[A])A
typeclassMin: [A](a: A, aa: A)(implicit evidence$2: Ordering[A])A

scala> typeclassMax(2.0, NaN)
res0: Double = NaN

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

Scala 2.13.1

Welcome to Scala 2.13.1 (OpenJDK 64-Bit Server VM, Java 1.8.0_222).
Type in expressions for evaluation. Or try :help.

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

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res0: Array[Double] = Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

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

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).min
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res2: Double = -0.0

scala> List(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res3: List[Double] = List(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

scala> List(2.0, 1.0, NaN, 3.0, 0.0, -0.0).max
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res4: Double = NaN

scala> List(2.0, 1.0, NaN, 3.0, 0.0, -0.0).min
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res5: Double = -0.0

scala> 2.0 max NaN
res6: Double = NaN

scala> 2.0 min NaN
res7: Double = NaN

scala> :paste
// Entering paste mode (ctrl-D to finish)

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

// Exiting paste mode, now interpreting.

typeclassMax: [A](a: A, aa: A)(implicit evidence$1: Ordering[A])A
typeclassMin: [A](a: A, aa: A)(implicit evidence$2: Ordering[A])A

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

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

Scala 2.13.1 + IEEE ordering

Welcome to Scala 2.13.1 (OpenJDK 64-Bit Server VM, Java 1.8.0_222).
Type in expressions for evaluation. Or try :help.

scala> import Ordering.Double.IeeeOrdering
import Ordering.Double.IeeeOrdering

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

scala> Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
res0: Array[Double] = Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

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

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

scala> List(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted
res3: List[Double] = List(-0.0, 0.0, 1.0, 2.0, 3.0, NaN)

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

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

scala> 2.0 max NaN
res6: Double = NaN

scala> 2.0 min NaN
res7: Double = NaN

scala> :paste
// Entering paste mode (ctrl-D to finish)

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

// Exiting paste mode, now interpreting.

typeclassMax: [A](a: A, aa: A)(implicit evidence$1: Ordering[A])A
typeclassMin: [A](a: A, aa: A)(implicit evidence$2: Ordering[A])A

scala> typeclassMax(2.0, NaN)
res8: Double = NaN

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

eed3si9n · 2020-01-02T20:49:42Z

@sjrd

Un-deprecate DeprecatedOrdering

I agree that just undeprecating DeprecatedOrdering seems like the way to go.

Change the implementation of x min y and x max y so that the special-case {Float,Double}.DeprecatedOrdering, in order to apply IEEE behavior.

~~I don't know if that's necessary since RichDouble's def max delegates to scala.math.max, which delegates to java.lang.Math.max.~~

@Ichoran gave me an example code.

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)
warning: there was one deprecation warning (since 2.13.0); for details, enable `:setting -deprecation' or `:replay -deprecation'
res1: Double = 2.0

Better the solution in a future where we can break bin compat: rewrite x min y so that they don't use 5 layers of abstraction that are slow and don't even work, along with all the other operations in RichXyz (I have a branch with that already).

I think the surprising thing about this is that x max y (IEEE ordering) does not match the result of List(x, y).max, which is consistent with Java and Scala 2.12 but surprising nonetheless. When we can break the bincompat in the future, maybe we should deprecate def max from RichDouble in favor of either calling math.max or Ordering[Double].max. The "math" part hopefully conveys that it's IEEE compliant.

martijnhoekstra · 2020-01-02T20:51:31Z

The "math" part hopefully conveys that it's IEEE compliant.

I wouldn't count on it.

eed3si9n · 2020-01-02T20:56:22Z

@martijnhoekstra You're right. I guess then rename to def ieeeMax?

martijnhoekstra · 2020-01-02T21:20:12Z

@martijnhoekstra You're right. I guess then rename to def ieeeMax?

Personally, I would see more in an ieee.max, but it's somewhat academic when the first opportunity for bincompat breakage is 3.2

Ichoran · 2020-01-02T22:02:33Z

Like it or not, the IEEE754 implementation of floating point numbers is ubiquitous. I do not think we can simply ignore it as inconvenient. Weighing down "max" with a bunch of extra letters basically just means that people won't use it and/or can't read math with it, at which point you'd be doing everyone a favor by just removing it entirely.

Every operation a □ b from ℝ² → ℝ has the property in IEEE754 mathematics that it is equivalent to

val oa = if (a.isNaN) None else Some(a)
val ob = if (b.isNaN) None else Some(b)
val oc = for { ra <- oa; rb <- ob } yield ra □ rb
oc.getOrElse(NaN)

IEEE754 doesn't define max explicitly, but given that everything else works this way, it's highly incongruent to have 2.0 min NaN return 2.0; unlike with all other binary operations, it silently fails to propagate error conditions.

IEEE754 does define a total ordering, and Java botches it, as do we. All the NaNs, of all types, go to the end, in random order. However, IEEE754 defines a total ordering where the negative NaNs come first, then the positive ones come after. Within the NaNs, quiet NaN is more extreme than the signaling NaNs. (It also makes -0.0 come before 0.0.) This is, unfortunately, really inconvenient because you now have to consider two different places where NaNs might appear in a sorted list. (This maintains the invariant that xs.map(x => -x).sorted is the same as xs.map(x => -x).reverse.)

People like to complain about IEEE754, but if you actually think carefully through all the issues, keeping in mind the hardware and data type constraints, and that speed and correctness are both very important, it's actually really hard to do any better. The standard isn't there by accident. It is hard to use correctly. This is an intrinsic awkwardness with the reals, compounded by the very necessary constraint to use finite precision.

So I think the correct thing to do is adhere to the standard whenever possible. If Ordering is supposed to be total and internally consistent, then Float and Double can't have one. (This is what Rust does; it only has a PartialOrd<f64>, not an Ordering<f64>.) If we admit partial orderings into Ordering, then the IEEE754 rules are fine as is; or we can allow inconsistency within different methods of Ordering.

NthPortal · 2020-01-03T04:30:19Z

java.util.Arrays, java.util.Collections, Scala 2.12.10, and Scala 2.13.1 with or without IEEE ordering all sorts Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0) to be Array(-0.0, 0.0, 1.0, 2.0, 3.0, NaN). << @NthPortal is there a corner case that sorts differently?

not that I'm aware of. I believe all of Scala's sorts end up deferring to j.u.Arrays.sort somewhere, and that (since it uses a Comparator) ends up calling Ordering#compare, which is not broken anywhere (because it requires you to return some Int).

eed3si9n · 2020-01-03T06:03:35Z

@Ichoran

Every operation a □ b from ℝ² → ℝ has the property in IEEE754 mathematics that it is equivalent to
val oa = if (a.isNaN) None else Some(a)
val ob = if (b.isNaN) None else Some(b)
val oc = for { ra <- oa; rb <- ob } yield ra □ rb
oc.getOrElse(NaN)

Thanks for this. If the notion of Double is like Option[DecimalNumber] as you portray, then I don't even know if it's appropriate to use Ordering for both the concept of sorting a list and a min x. In that world, shouldn't Array(2.0, 1.0, NaN, 3.0, 0.0, -0.0).sorted also return Array(NaN) so that its max and min value would match 2.0 max NaN?

Java did the right thing by separating the responsibilities to java.util.Collections and java.lang.Math with this regard, whereas in Scala it's not clear what we mean when we say min or max.

a min b should use IEEE ordering, like java.lang.Math, regardless of the Ordering[Double] that's in scope.
List#min should return List#sorted.head, like java.util.Collections.
Given the different semantics, RichDouble#min and List#min should have different method names, and provide migration path.

Ichoran · 2020-01-03T17:29:20Z

@eed3si9n - That's a reasonable strategy that doesn't compromise anything too much.

Given the heavy numeric use of min and max, and the importance of naming consistency, min and max should require a Numeric while some other name (maybe minimum and maximum) can be used to pick out the first and last things in sort-order.

But how do we get there, and what do we do in the short run to make List(1.0, -1.0).sorted not ominously announce a deprecation without actually supplying any actionable information? (And avoid silently changing the meaning of numeric code written for 2.12 and before?)

eed3si9n · 2020-01-03T19:14:36Z

But how do we get there, and what do we do in the short run to make List(1.0, -1.0).sorted not ominously announce a deprecation without actually supplying any actionable information? (And avoid silently changing the meaning of numeric code written for 2.12 and before?)

1. Undeprecate `DeprecatedOrdering` 2. Change the implementation of `x min y` and `x max y` such that in all instances they use IEEE ordering, like `java.lang.Math` (This will match the 2.12 semantics) 3. Add `x minimum y` and `x maximum y` operators that would use the same ordering as sorting order `compare(x, y)` 4. Add `List#minimum` and `List#maximum` that returns the `List#sorted.head` and `List#sorted.last` 5. Deprecate `List#max` and `List#min` after pointing them to `List#maximum` and `List#minimum` (We get to deprecate the methods whose implementation must change from 2.12)

odersky · 2020-01-29T16:14:28Z

I think a -migration warnings could be acceptable, if it is carefully worded. Something like: The run-time semantics of ordering has changed in 2.13 from IEEE ordering to a total ordering. The new ordering is the same as what Java uses. The difference affects only NaN operands, normal values are unaffected. If you need to maintain the old IEEE ordering you can do this by importing <name of overriding import> I am not sure this is all correct, but you get the idea...

…

On Wed, Jan 29, 2020 at 4:58 PM Seth Tisue ***@***.***> wrote: rules out a migration warning as completely unacceptable Disagree — the migration warning would only appear with -Xmigration enabled, and people are only expected to use that temporarily, when they are in the process of upgrading. (And in practice, most people probably don't/didn't bother.) I take Martin's meaning to have been, "any kind of warning (with typical settings)" — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#11844?email_source=notifications&email_token=AAGCKVWOSSDBV4S4FHVPCB3RAGRRLA5CNFSM4KCDWJNKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKHXEAI#issuecomment-579826177>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGCKVQDW2FIUFJ337JI2SDRAGRRLANCNFSM4KCDWJNA> .

-- Martin Odersky EPFL and Lightbend

szeiger · 2020-01-29T17:28:11Z

Regarding the long-term evolution of these features, I also think that the 2.13 default TotalOrdering is the correct choice. IeeeOrdering is a hack that may be useful in some cases but it is not a valid Ordering. The choice of total ordering (which is what Ordering is supposed to represent) is not what's wrong with the current design.

I rather expect to see changes in the following areas:

Some operations (e.g. sorted) should only require a PartialOrdering instead of an Ordering
Numeric should not extend Ordering

Ichoran · 2020-01-29T18:50:18Z

The migration warning is also hidden change in behavior, which is something we were trying to avoid.

I agree on the long-term migration strategy. It's just a shame that it will take the better part of a decade to get there given our current plans.

SethTisue · 2020-01-29T20:39:12Z

we did not mention -Xmigration in the 2.13.0 release notes. that was an oversight

when we release 2.13.2 in a month or so, I can highlight it in those notes, and I can also go back and add it to the 2.13.0 notes

w/r/t migration warnings, I think we fell into a vicious cycle where we didn't use @migration much or promote -Xmigration ourselves, so people didn't use it, and since we didn't expect people to use it, the cycle went around. I think we can break out of this cycle, to a reasonable extent (some people don't read release notes or follow recommendations, but 🤷‍♂️)

eed3si9n · 2020-02-12T02:08:54Z

Consensus seems to be forming around Stefan's -Xmigration suggestion for Scala 2.13.2:

If we want to ensure that code like List(1.0, -1.0).sorted continues to work without a deprecation warning we can replace the deprecation warning by a migration warning.

I guess this would be about the only thing we could do in 2.13.x patch release.

The next step then is crafting the migration warning. I personally think we should avoid mentioning Java, since I didn't understand what was meant by that exactly. What do you think of the following?

The runtime semantics of ordering has changed in 2.13 from IEEE ordering to a total ordering. The new ordering is the same as the way collections are sorted, with NaN at the end. The difference affects only NaN and -0.0 operands, and normal values are unaffected. If you need to maintain the IEEE ordering you can do so by importing scala.Ordering.Double.IeeeOrdering.

martijnhoekstra · 2020-02-12T10:29:25Z

I'd prefer a phrasing that doesn't imply NaN and -0.0 aren't normal values.

odersky · 2020-02-12T11:20:27Z

I think the migration message is very good. Specifically, "normal values are unaffected" is exactly right 👍

SethTisue · 2020-02-12T15:35:08Z

Martijn, I don't understand the thought behind your objection?

(perhaps "ordinary numbers"?)

Jasper-M · 2020-02-12T15:56:44Z

We don't want to discriminate any Doubles just cause they're a bit weird.

sjrd · 2020-02-12T15:58:37Z

One can argue that NaN is not a normal value.

But pretending that -0.0 is not normal is a stretch, given that simple, valid computations like -6.0 % 3.0 can produce -0.0.

Just use "other values are not affected" instead of "normal values", and everyone should be happy.

martijnhoekstra · 2020-02-12T16:00:56Z

@SethTisue I don't think that calling them normal values makes things clearer than just listing the exceptions. For example, positive and negative infinity are handled the same. Are they normal values? Would you need to check after reading the migration warning? Something like the following side-steps that:

The behaviour of the default (implicit) Ordering on Double has changed in 2.13 from IEEE ordering to a total ordering. The new ordering differs from the old only for some comparisons with NaN and -0.0 operands. If you need to maintain the IEEE ordering you can do so by importing scala.Ordering.Double.IeeeOrdering. See that orderings scaladoc for details.

It IMO also vaguely suggests you are doing something wrong if you are using these abnormal values, which I thought wasn't the intent when I made the comment, but I'm not sure of anymore now that I see Martin's reply.

Ichoran · 2020-02-12T21:45:45Z

The message sounds good, but actually doesn't quite convey the right thing because the compare method, which is the heart of any Ordering, doesn't actually change. It's the other comparison methods.

So maybe, if I understand correctly how things work (which is not assured):

The behavior of the comparison operations provided by the default (implicit) ordering
on Double has changed in 2.13.  Comparison operators now match `compare` instead
of giving IEEE754 behavior for -0.0 and NaN.  Import Ordering.Double.IeeeOrdering to
retain the previous behavior.

eed3si9n · 2020-02-13T17:52:34Z

@Ichoran

Comparison operators now match compare instead of giving IEEE754 behavior for -0.0 and NaN.

"Comparison operators" might be misleading since reader might assume that Double#< has changed. Not sure if this makes it clearer, but I tried listing them out:

scala> List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).min
                                             ^
       warning: object DeprecatedDoubleOrdering in object Ordering has changed semantics in version 2.13.0:
       The behavior of the comparison operations provided by the default (implicit) ordering
       on Double has changed in 2.13. Methods such as `lt`, `min`, and `equiv` now match `compare`
       instead of giving IEEE754 behavior for -0.0 and NaN.
       Import Ordering.Double.IeeeOrdering to retain the previous behavior.
val res0: Double = -Infinity

scala> import Ordering.Double.IeeeOrdering
import Ordering.Double.IeeeOrdering

scala> List(0.0, 1.0, 0.0 / 0.0, -1.0 / 0.0).min
val res1: Double = NaN

Fixes scala/bug#11844 Ref scala/bug#10511 Ref scala#6410 Ref scala#76 This change the deprecation of `DeprecatedDoubleOrdering` to a migration warning instead to avoid `List(1.0, -1.0).sorted` giving deprecation warning. This also provides some documentation on the ordering instances in Scaladoc.

eed3si9n · 2020-02-15T20:14:36Z

Here's my PR - scala/scala#8721

NthPortal · 2020-02-16T04:35:08Z

thanks for taking care of that @eed3si9n!

Fixes scala/bug#11844 Ref scala/bug#10511 Ref scala#6410 Ref scala#76 This change the deprecation of `DeprecatedDoubleOrdering` to a migration warning instead to avoid `List(1.0, -1.0).sorted` giving deprecation warning. This also provides some documentation on the ordering instances in Scaladoc.

NthPortal · 2020-04-16T20:45:33Z

we need to perform the same fix for scala.math.Equiv (which no one uses, so we didn't notice)

lrytz · 2020-04-17T07:23:28Z

@SethTisue for 2.13.2?

SethTisue · 2020-04-17T13:14:31Z

sure

NthPortal · 2020-04-17T14:00:04Z

for reference, the previous implicit instances of Equiv for floating point types were the universal equiv, so keep that in mind when writing migration text (if any). we might just need to remove the @deprecated annotation

SethTisue · 2020-05-01T02:39:18Z

for reference, the previous implicit instances of Equiv for floating point types were the universal equiv, so keep that in mind when writing migration text (if any). we might just need to remove the @deprecated annotation

I found that the default behavior did change between 2.12 and 2.13, so it appears to me that we do need the migration warning:

2.12.11:

scala 2.12.11> implicitly[Equiv[Float]]
res0: Equiv[Float] = scala.math.Equiv$$anon$2@d229912

scala 2.12.11> (res0.equiv(Float.NaN, Float.NaN), res0.equiv(0f, -0f))
res1: (Boolean, Boolean) = (false,true)

2.13.2:

scala 2.13.2> implicitly[Equiv[Float]]
                        ^
              warning: object DeprecatedFloatEquiv in object Equiv is deprecated (since 2.13.0): There are multiple equivalences for Floats (Equiv.Float.TotalEquiv, Equiv.Float.IeeeEquiv). Specify one by using a local import, assigning an implicit val, or passing it explicitly. See the documentation for details.
val res0: Equiv[Float] = scala.math.Equiv$DeprecatedFloatEquiv$@41aebbb4

scala 2.13.2> (res0.equiv(Float.NaN, Float.NaN), res0.equiv(0f, -0f))
val res1: (Boolean, Boolean) = (true,false)

I'm working on a PR.

but give a migration warning, since the default is now the strict one rather than the IEEE one we should have done this when we did the same for `Ordering`; it was just an oversight context: scala/bug#11844

SethTisue · 2020-05-01T04:28:12Z

PR: scala/scala#8945

NthPortal · 2020-05-01T13:41:43Z

thank you Seth 💜

eed3si9n mentioned this issue Jan 2, 2020

Ordering methods are inconsistent for Double and Float #10511

Closed

dwijnand added the library label Jan 2, 2020

eed3si9n mentioned this issue Jan 2, 2020

Un-deprecate default floating point Orderings scala/scala#8612

Closed

eed3si9n added the has PR label Jan 2, 2020

lrytz assigned eed3si9n Feb 3, 2020

eed3si9n mentioned this issue Feb 15, 2020

Un-deprecate default floating point Orderings; issue migration warning instead under -Xmigration scala/scala#8721

Merged

SethTisue closed this as completed in scala/scala#8721 Feb 18, 2020

SethTisue mentioned this issue May 1, 2020

un-deprecate default floating point Equivs scala/scala#8945

Merged

Jasper-M mentioned this issue Aug 6, 2020

Array.min in 2.12.12 vs 2.13.3 #12107

Closed

List(1.0, -1.0).sorted gives a deprecation warning #11844

List(1.0, -1.0).sorted gives a deprecation warning #11844

Comments

eed3si9n commented Jan 2, 2020 • edited

steps

problem

expectation

notes

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

Ichoran commented Jan 2, 2020 • edited

eed3si9n commented Jan 2, 2020

sjrd commented Jan 2, 2020

eed3si9n commented Jan 2, 2020 • edited

summary

java.util.Arrays sorting primitive double

java.util.Collections sorting boxed java.lang.Double

java.lang.Math

Scala 2.12.10

Scala 2.13.1

Scala 2.13.1 + IEEE ordering

eed3si9n commented Jan 2, 2020 • edited

martijnhoekstra commented Jan 2, 2020

eed3si9n commented Jan 2, 2020

martijnhoekstra commented Jan 2, 2020

Ichoran commented Jan 2, 2020 • edited

NthPortal commented Jan 3, 2020

eed3si9n commented Jan 3, 2020

Ichoran commented Jan 3, 2020

eed3si9n commented Jan 3, 2020 • edited

odersky commented Jan 29, 2020 via email

szeiger commented Jan 29, 2020

Ichoran commented Jan 29, 2020

SethTisue commented Jan 29, 2020

eed3si9n commented Feb 12, 2020

martijnhoekstra commented Feb 12, 2020

odersky commented Feb 12, 2020

SethTisue commented Feb 12, 2020

Jasper-M commented Feb 12, 2020

sjrd commented Feb 12, 2020

martijnhoekstra commented Feb 12, 2020

Ichoran commented Feb 12, 2020

eed3si9n commented Feb 13, 2020

eed3si9n commented Feb 15, 2020

NthPortal commented Feb 16, 2020

NthPortal commented Apr 16, 2020

lrytz commented Apr 17, 2020

SethTisue commented Apr 17, 2020

NthPortal commented Apr 17, 2020

SethTisue commented May 1, 2020

SethTisue commented May 1, 2020

NthPortal commented May 1, 2020

eed3si9n commented Jan 2, 2020 •

edited

Ichoran commented Jan 2, 2020 •

edited

eed3si9n commented Jan 2, 2020 •

edited

eed3si9n commented Jan 2, 2020 •

edited

Ichoran commented Jan 2, 2020 •

edited

eed3si9n commented Jan 3, 2020 •

edited