Groovy
  1. Groovy
  2. GROOVY-3309

comparisons with NaN should always return false

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.5.7, 1.6-rc-2
    • Fix Version/s: 3.0
    • Component/s: groovy-runtime
    • Labels:
      None
    • Environment:
      os x, jse 5 and 6

      Description

      The following assertions should all pass; the third fails.

      assert !(Double.NaN == 0)
      assert !(Double.NaN < 0)
      assert !(Double.NaN > 0)
      

      A NaN should also not equal a NaN.

      assert Double.NaN != Double.NaN

        Issue Links

          Activity

          Hide
          Roshan Dawrani added a comment -

          Comparisons that groovy is doing is through calls to Double.compare(..) in this case:

          In Java too:
          Double.compare(Double.NaN, 0) //returns 1, indicating NaN > 0
          Double.compare(Double.NaN, Double.NaN) //returns 0, indicating NaN and NaN are equal.

          However:
          Double.NaN < 0 //returns false
          Double.NaN > 0 //returns false
          Double.NaN == Double.NaN // returns false

          So, does it mean that even in Groovy, if Double.NaN, Float.NaN are involved, they should be treated differently before the comparison reaches Double.compare(..) / Float.compare(..), etc?

          Show
          Roshan Dawrani added a comment - Comparisons that groovy is doing is through calls to Double.compare(..) in this case: In Java too: Double.compare(Double.NaN, 0) //returns 1, indicating NaN > 0 Double.compare(Double.NaN, Double.NaN) //returns 0, indicating NaN and NaN are equal. However: Double.NaN < 0 //returns false Double.NaN > 0 //returns false Double.NaN == Double.NaN // returns false So, does it mean that even in Groovy, if Double.NaN, Float.NaN are involved, they should be treated differently before the comparison reaches Double.compare(..) / Float.compare(..), etc?
          Hide
          Jochen Theodorou added a comment - - edited

          compareTo supports returning a positive value (for example +1), a negative value (for example -1) or zero. Now if Double.NAN<=>0 must not return 0, then that leaves +1 or .-1. If we also say Double.NAN<=>0 must not return -1, then it must return +1 or throw an exception. Andy seems to suggest that compare returns neither +1, 0 nor -1, which leaves the exception as only possibility. The javadoc of compareTo says:

          • Double.NaN is considered by this method to be equal to itself and greater than all other double values (including Double.POSITIVE_INFINITY).
          • 0.0d is considered by this method to be greater than -0.0d.

          This ensures that Double.compareTo(Object) (which forwards its behavior to this method) obeys the general contract for Comparable.compareTo, and that the natural order on Doubles is consistent with equals.

          So they wanted to define an order on Double including NAN to avoid exceptions and decided that NAN is to be handled as a special value bigger than POSITIVE_INFINITY. This has the advantage, that if you sort a list of doubles for example, then you have to check for invalid doubles before. They are not invalid because they are NAN, they would be invalid if they would throw an exception while comparing.

          You may say that a comparable Double.NAN is mathematically more correct, on the other hand I can easily define a correct math for this.

          But it seems there is a problem, because a double NaN and a Double NaN behave different.If I do Double.NaN==Double.NaN in Java, then it will return false. This is according to IEEE 754 correct. The only comparision of Double.NaN with another double that return true is Double.NaN!=Double.NaN. So Double.compareTo behaves much different from normal primitive double operations. If we want to follow the primitive operations we have to adapt the Groovy version of comparing Double numbers.

          And I suggest to follow the primitive doubles of Java here

          Show
          Jochen Theodorou added a comment - - edited compareTo supports returning a positive value (for example +1), a negative value (for example -1) or zero. Now if Double.NAN<=>0 must not return 0, then that leaves +1 or .-1. If we also say Double.NAN<=>0 must not return -1, then it must return +1 or throw an exception. Andy seems to suggest that compare returns neither +1, 0 nor -1, which leaves the exception as only possibility. The javadoc of compareTo says: Double.NaN is considered by this method to be equal to itself and greater than all other double values (including Double.POSITIVE_INFINITY). 0.0d is considered by this method to be greater than -0.0d. This ensures that Double.compareTo(Object) (which forwards its behavior to this method) obeys the general contract for Comparable.compareTo, and that the natural order on Doubles is consistent with equals. So they wanted to define an order on Double including NAN to avoid exceptions and decided that NAN is to be handled as a special value bigger than POSITIVE_INFINITY. This has the advantage, that if you sort a list of doubles for example, then you have to check for invalid doubles before. They are not invalid because they are NAN, they would be invalid if they would throw an exception while comparing. You may say that a comparable Double.NAN is mathematically more correct, on the other hand I can easily define a correct math for this. But it seems there is a problem, because a double NaN and a Double NaN behave different.If I do Double.NaN==Double.NaN in Java, then it will return false. This is according to IEEE 754 correct. The only comparision of Double.NaN with another double that return true is Double.NaN!=Double.NaN. So Double.compareTo behaves much different from normal primitive double operations. If we want to follow the primitive operations we have to adapt the Groovy version of comparing Double numbers. And I suggest to follow the primitive doubles of Java here
          Hide
          John Wagenleitner added a comment -

          Added code tags and also want to mention another test case from GROOVY-4018 that should pass:

          assert !(Double.NaN)
          
          Show
          John Wagenleitner added a comment - Added code tags and also want to mention another test case from GROOVY-4018 that should pass: assert !( Double .NaN)
          Hide
          John Wagenleitner added a comment -

          Just want to point out that the 3rd case that was reported to fail will pass if a literal double is used instead of just 0.

          assert !(Double.NaN > 0d)
          
          double d;  // just to show the uninitialized value works (0.0d)
          assert !(Double.NaN > d)
          

          I think for the NaN to NaN comparison not only does Double#compareTo but also Double#equals also returns true. So think the following are all reasonable:

          Double d1 = Double.NaN
          Double d2 = Double.NaN
          
          assert d1 == d2
          assert !d1.is(d2)
          assert !d2.is(d1)
          

          Since == usually is compareTo or equals, it seems that to do the same route with primitives the choice would be to use is.

          Show
          John Wagenleitner added a comment - Just want to point out that the 3rd case that was reported to fail will pass if a literal double is used instead of just 0. assert !( Double .NaN > 0d) double d; // just to show the uninitialized value works (0.0d) assert !( Double .NaN > d) I think for the NaN to NaN comparison not only does Double#compareTo but also Double#equals also returns true. So think the following are all reasonable: Double d1 = Double .NaN Double d2 = Double .NaN assert d1 == d2 assert !d1.is(d2) assert !d2.is(d1) Since == usually is compareTo or equals , it seems that to do the same route with primitives the choice would be to use is .

            People

            • Assignee:
              Unassigned
              Reporter:
              Andy Fyfe
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:

                Development