catsSyntaxEq returns false for equality of positive and negative zero in Option

[charlesandrews]

With versions:

• Scala 3.3.3
• cats-core 2.13.0

When comparing Options which contain doubles, catsSyntaxEq === unexpectedly returns false for comparisons of 0.0 and -0.0

scala> 0.0 === -0.0
val res2: Boolean = true

scala> Option(0.0) === Option(-0.0)
val res0: Boolean = false

This does not match Scala's behavior of

scala> Option(0.0) == Option(-0.0)
val res1: Boolean = true

[satorg]

Thank you for the report!

The issue does exist indeed. The culprit is DoubleOrder:

cats/kernel/src/main/scala/cats/kernel/instances/DoubleInstances.scala

Lines 37 to 44 in 08f88ea

	class DoubleOrder extends Order[Double] with Hash[Double] {
	def hash(x: Double): Int = x.hashCode()
	def compare(x: Double, y: Double): Int =
	java.lang.Double.compare(x, y)
	override def eqv(x: Double, y: Double): Boolean = x == y
	override def neqv(x: Double, y: Double): Boolean = x != y

When doubles are compared directly with ===, then eqv is called. Which uses the built-in == for primitives.

However, when Option-s are compared, then compare is used instead:

cats/kernel/src/main/scala/cats/kernel/instances/OptionInstances.scala

Lines 49 to 60 in 08f88ea

	class OptionOrder[A](implicit A: Order[A]) extends Order[Option[A]] {
	def compare(x: Option[A], y: Option[A]): Int =
	x match {
	case None =>
	if (y.isEmpty) 0 else -1
	case Some(a) =>
	y match {
	case None => 1
	case Some(b) => A.compare(a, b)
	}
	}
	}

which delegates to DoubleOrder.compare, which, in turn – to java.lang.Double.compare.

In fact, the same issue can be reproduced without any Cats at all:

scala> 0.0 == -0.0
val res0: Boolean = true
                                                                                                                                                                                                        
scala> (0.0: java.lang.Double) == (-0.0: java.lang.Double)
val res1: Boolean = false

Which in turn can be reduced to pure Java:

jshell> 0.0 == -0.0
$1 ==> true

jshell> ((Double) 0.0).equals(-0.0)
$2 ==> false

[satorg]

Interestingly, that there are corresponding laws that would fail for the pair of values 0.0 and -0.0:

cats/kernel-laws/shared/src/main/scala/cats/kernel/laws/PartialOrderLaws.scala

Lines 51 to 54 in 08f88ea

	def partialCompare(x: A, y: A): IsEq[Boolean] = {
	val c = E.partialCompare(x, y)
	(((c < 0) == E.lt(x, y)) && ((c == 0) == E.eqv(x, y)) && ((c > 0) == E.gt(x, y))) <-> true
	}

cats/kernel-laws/shared/src/main/scala/cats/kernel/laws/OrderLaws.scala

Lines 32 to 35 in 08f88ea

	def compare(x: A, y: A): IsEq[Boolean] = {
	val c = E.compare(x, y)
	(((c < 0) == E.lt(x, y)) && ((c == 0) == E.eqv(x, y)) && ((c > 0) == E.gt(x, y))) <-> true
	}

For example:

scala> cats.kernel.laws.OrderLaws[Double].partialCompare(0.0, -0.0)
val res0: cats.kernel.laws.IsEq[Boolean] = IsEq(false,true)

scala> cats.kernel.laws.OrderLaws[Double].compare(0.0, -0.0)
val res1: cats.kernel.laws.IsEq[Boolean] = IsEq(false,true)

But they don't – probably because ScalaCheck never generates those values together in the checks. Maybe it never generates -0.0 whatsoever.

[johnynek]

Could we fix this by changing the eqv implementation to use Double.compare(x, y) == 0?

Also, I wonder if we could make a new Gen[Double] that does:

val customDoubleGen = Gen.frequency(
  20 -> defaultDoubleGen,
   1 -> Gen.oneOf(-0.0, 0.0, Double.PositiveInfinity, Double.NegativeInfinity)
)

Or something to ensure that we are getting some or the hard values in there (I think we can exclude NaN, but maybe we shouldn't, although the default NaN ordering on the IEEE754 isn't lawful).

[satorg]

Could we fix this by changing the eqv implementation to use Double.compare(x, y) == 0?

I guess we could, but I'm wondering how it would affect the performance. Double.compare is apparently slower – like, 3x times slower for values that are definitely different and even worse for values that might be equal. But the overall impact would depend on a particular case, of course. Maybe it wouldn't that bad after all.

Also, I wonder if we could make a new Gen[Double] that does:

This one seems trickier: from the suggested numbers, the probability of 0.0 is slightly higher than $\frac{1}{21*4} \approx \frac{1}{80}$. The same for -0.0. Consequently, the probability that both 0.0 and -0.0 will be observed by compare (or partialCompare), in any order, is $\frac{1}{80} \times \frac{1}{80} \times 2=\frac{1}{3200}$. Given that Cats tests evaluate every property for 100 times (by default), the probability that we'll see 0.0 and -0.0 tested together within one property at least once is just $1 - (1 - \frac{1}{3200})^{100} \approx \frac{1}{32}$. (Hopefully, I did the math correctly)

We can certainly try to tweak some numbers, but the overall approach doesn't seem robust to me.

Or something to ensure that we are getting some or the hard values in there (I think we can exclude NaN, but maybe we shouldn't, although the default NaN ordering on the IEEE754 isn't lawful).

I'm not sure we can do it in a generic way. Perhaps, we could add a dedicated rule for that case, but it would only make sense for Double and Float types. I.e. we would have to test whether A in OrderLaws is Double or Float and then verify, or just pass otherwise.