(Double.NaN min 0.0) yields 0.0, should be NaN

[scabug]

[Original report is classified as out of scope; ticket is open for (NaN min 0) wrongness.]

*import* scala.math.Ordering.Double._

Hi Scala folks!

I find something in Scala, that made me searching the web, reading documentations and rummaging in the sources of Scala and trying many things. So far so good, but I'm still wondering why is the following expression *true*:

max(Double.NaN, 0) != min(Double.NaN)

I found out that NaN in Ordering.Double is bigger than everything else, even that _PositivInfinity_! And NaN is equal to it self. The implementation in java.lang.Double.compare have the same behaviour.

The other implementation is the the native one, based on IEEE. (You know <, >= and so on.) This one says that every comparison with NaN returns *false*, even NaN==NaN is *false*.

I think is a philosophical discussion what the better way is. I prefer IEEE but that doesn't matter.

The point is the code snipped at the top with min and max. I can not say what is bigger. NaN or 0? I also can't say what is smaller. NaN or 0? I have the same problem, so I expected the same result of both methods. Currently max returns always NaN and min the other number (0 in my example).

Now I see four possible solutions for this inconsistency:
(I tried to mark pro and contra, but they are not weighted. Some points are definitely stronger than others.)

• Throwing an exception every time a methods in Ordering.Double gets an NaN.
  (+) That is a hyper correct dealing with NaN
  (-) and a hidden trap for many programmers, because this solution would be far away from the native handling with NaN and the use of an Ordering with implicit parameters isn't visible for starting programmers.
  (-) Throwing an exception for normal program flow? Doesn't sounds good.
  (-) Currently fine working code could throw suddenly exceptions.
• Keep as it is and add at least one comment in the documentation.
  (-) Leave the problem
  (+) Leave the code
• Using an arithmetic expression to solve min and max for Double:

  def max(a, b)=mean(a, b) + abs(a - b)

(+) That means the result is NaN and would make NaN more dominant like in other arithmetic operations.
(-) I fear that someone had used min and max to sort something.

  def sorted(a, b)=(max(a, b), min(a, b))

A single NaN could duplicate its self this way. Old code is may not cable of handling this behaviour.
(-) This changes treats to change the min and max methods in TraversableOnce to:

  def max[B >: A](implicit cmp: Ordering[B]):A = reduce(cmp.max)

(+) But I think that would be good changes anyway.

• Ordering.Double should be direct child of ParticalOrdering
  (+) That's an other hyper correct dealing NaN.
  (-) RichDouble or its parent ScalaNumberProxy need Double as a{Numeric and Numeric depends on Ordered. I think that can be resolved with more re-factoring, but it would be hard work.
  (+) It's possible to mark the old way as deprecated. Every becomes a warning, if he use min or max on Doubles.
  (-) Many people (like me) want to use the min and max methods in TraversableOnce, so the methods have to handle this new situation.
  A very naive implementation of max could be:

  def max[B >: A](implicit cmp: ParticalOrdering[B]):A =
    first( x => forall(cmp.lteq(_, x)) && forall(cmp.tryCompare(_, x) != None))

This would have to throw an exception if NaN part of the collection.
(+) But maybe that expand the use of ParticalOrdering.

You see, I'm very focused on min and max, that's because I can't decide what the best way is to deal with NaN and <, >=. Solution number three gives a good reason why min and max returns what they do and works around <, >=.
Maybe it is possible for solution number three get the (+) with deprecated of solution number four, but I have no idea how.

My favourite solution is number three. What do you think about it?

Sincerely,
D. Jentsch

[scabug]

Imported From: https://issues.scala-lang.org/browse/SI-5104?orig=1
Reporter: Jentsch (jentsch)

[scabug]

@paulp said:
This exceeds what can be addressed in a ticket. I suggest taking it up on a mailing list.

[scabug]

@dlwh said:
It looks like IEEE demands this behavior:

http://www.itu.dk/~sestoft/javaprecisely/java-floatingpoint.pdf

min of anything with NaN is NaN and max of anything with NaN is NaN.

Also by spec, (NaN != NaN) == true.

However, note that:

scala> math.min(0.0,Double.NaN)
res16: Double = NaN

scala> (0.0 min Double.NaN)
res17: Double = 0.0

which is probably bad. Should I open a separate ticket for this?

[scabug]

@paulp said:
I'll re-open this one.

[scabug]

Jentsch (jentsch) said:
So what now? Should I fix it and push a patch to git hub? (Sorry I'm in this open source business)

[scabug]

@khernyo said:
Here is a patch which fixes this and some related NaN ordering inconsistencies: https://github.com/szabolcsberecz/scala/commit/460bbc1276fb4ba83b9bcbdc7f7ba475b352b7c6

[scabug]

@khernyo said:
fixed in 460bbc1276fb4ba83b9bcbdc7f7ba475b352b7c6