Improve floating point IEEE Ordering consistency

[NthPortal]

Make Ordering#compare(0.0, -0.0) consistent with
the IEEE specification for floating point Orderings.

I randomly noticed this was not consistent with the IEEE spec (or with lt, gt, etc.), but could be.

[sjrd]

What about NaN arguments, though? There is no correct (consistent) answer for that.

IMO this painfully highlights that an Ordering must be total, and that IeeeOrdering cannot be an Ordering, even an "unreasonable but sometimes useful" one.

I would go as far as deprecating IeeeOrdering altogether. Remind me when that's considered useful, again?

[NthPortal]

What about NaN arguments, though? There is no correct (consistent) answer for that.

Yup. There's unfortunately not anything to do about that. I think the current behavior is the best we can get.

I would go as far as deprecating IeeeOrdering altogether. Remind me when that's considered useful, again?

  trait FloatIsFractional extends Fractional[Float] {
    // ...
  }
  implicit object FloatIsFractional extends FloatIsFractional with Ordering.Float.IeeeOrdering

I don't remember why we decided to do that; the best I can come up with are these two comments:

• bug#10511 Make Ordering methods consistent #6323 (comment)
• bug#10511 Make Ordering methods consistent #6323 (review)

[Ichoran]

@sjrd - The problem is that we have an insufficient number of abstractions: we only have Ordering not Min and Max (or Extremal).

IEEE754 insists that NaN max 2.5 and NaN min 2.5 are both NaN. This is entirely consistent with NaN's Option-like error capabilities. It also insists that NaN < 2.5 and 2.5 < NaN are both false.

Because we don't have Extremal, we can't distinguish between sorting, where we need a total order, and extreme values, where we don't (and instead expect to get an error signal). Therefore, the decision, regrettably, has to go to the user.

In other words, one would think that all these things are true:

a max b == Seq(a, b).max
a min b == Seq(a, b).min
Seq(a, b).sorted.map(_.toString) == Seq(b, a).sorted.map(_.toString)

but they can't be with the same rules on how floating-point numbers compare, and we have only a single abstraction instead of two.

[NthPortal]

at the end of the day, whatever we decide to do wrt the default ordering and the structure of the library, this is a strict improvement to IEEE orderings imo

[Ichoran]

@NthPortal - But compare isn't defined by IEEE754, and because it doesn't have an error state, it is required to do its best to act like a total ordering; given that we sort NaN to the end, it seems a lot more like the suggested total ordering (https://en.wikipedia.org/wiki/IEEE_754#Total-ordering_predicate) than the regular comparison operations, which makes the Java ordering fine w.r.t. +-0 (but not fine w.r.t. the various NaNs).

(Aside: we can declare NaN to be after anything, but then we can't implement a < b as a.compare(b) < 0 but rather a.compare(b) < 0 && b.compare(a) > 0 or somesuch. But we don't do that, probably because of the runtime overhead to catch one very particular special case.)

[NthPortal]

it is required to do its best to act like a total ordering

and it does. a total ordering does not require that equivalent elements be identical, afaik (or if you think of it differently, -0.0 and 0.0 are identical mathematically)

[Ichoran]

@NthPortal - Right, a total ordering can have equivalence classes. And you can declare a total ordering that does not respect the equivalence classes defined by some alternate ordering.

My point, though, is that the same total order that declares that -0.0 comes before 0.0 also declares that -qNaN < -sNaN (all of them) < actual numbers < sNaN (all of them) < qNaN, but we're not doing that; instead we're grouping all the NaNs into a single equivalence class. The other one, which isn't a total order, declares that 0.0 and -0.0 are equal, and doesn't say where NaN should go if you sort them. Since we don't sort NaN where they say to, I don't think it's correct that having -0.0 before 0.0 agrees better with IEEE754; it just makes us try to agree more with the IEEE754 total ordering, which we already fail at.

[SethTisue]

We need to decide whether to merge this for 2.13.2.

It appears to me that it is mergeable, or not, separately from the other open Ordering PRs (#8613, #8625), and that Seb's suggestion to deprecate IeeeOrdering is something that would happen in a separate PR, if it happens.

@NthPortal do you want to respond to Rex's last comment?

@Ichoran I confess to not understanding your last comment. in particular, I'm not able to follow the thinking behind "Since we don't sort NaN where they say to, I don't think it's correct that having -0.0 before 0.0 agrees better with IEEE754". as it stands, @NthPortal's "this is a strict improvement to IEEE orderings imo" is also where I stand... unless you can make your objection crystal-clear?

[Ichoran]

@SethTisue - My argument is as follows.

IEEE754 defines the behavior of a < relationship, in which neither -0 < 0 nor 0 < -0 (that is, zeros of all types form an equivalence class). Furthermore NaN is unordered; if a NaN touches <, the answer is always false (not because it's greater or equal, but because it's undefined).

IEEE754 also defines a total ordering over all bit patterns that can appear: a particular sequence of -NaN values (because there are actually 8388608 different negative NaNs), then -Inf, then finite numbers, with -0 appearing before 0, then Inf, then a sequence of positive NaN values (mirroring the negative ones). There are no equivalence classes.

This total ordering is dramatically different from what Java does for sorting, which is based on compare. compare puts all NaN values into an equivalence class at the end (i.e. compare(x, y) returns 0 if both are NaN, and otherwise returns 1 or -1 to make the NaN seem bigger, even if it's negative), but it insists (oddly) that -0 comes before 0.

By having IeeeOrdering.compare lump -0 and 0 into one bin, it makes it a little less like ordinary compare, but it still sorts all the NaNs to the end, just like in Java (because you have to put them somewhere)!

So we add mandatory extra computation on every compare, but still don't give either the IEEE total ordering or agreement with <. We're just not using IEEE754 < or IEEE754 total order for compare, either before or after the change.

Adding a likely runtime penalty for all compare operations, while making IeeeOrdering agree less well with Java compare (which otherwise it is identical to), but not actually giving the IEEE754 total order, does not seem like an improvement to me.

Before, compare doesn't match <. After, compare still doesn't match <. (But now only on NaN.)

Before, compare matches Java. After, compare doesn't match Java.

Before, compare doesn't match the IEEE754 total order. After, it still doesn't.

I just don't see this as better in any meaningful way. It's different. And possibly slower, which is always bad for things like this, so should be tested.

[SethTisue]

@Ichoran I understand. Thank you.

Also, I'm increasingly convinced that we shouldn't be changing the runtime behaviors anymore in 2.13.x unless they're Just Plain Wrong. Whether Rex's argument would convince me for a 2.14 or 3.0 decision, I don't know. But in the 2.13.x context, it's enough: I'm convinced this would be a design change and not just a bugfix.