`Equals` with NaN values (IEEE vs. reflexivity)

[CodesInChaos]

The built in floating point types compare NaN as unequal when using == and != (following IEEE semantics) but compare it as equal when using the Equals method.

Your floating point based types currently use IEEE semantics even for Equals. I suggest using the same behaviour as the built in types in your floating point based types like vectors or quaternions.

The MSDN documentation of Equals contains an exception that allows A.Equals(A) to return false on floating point types, so you don't strictly violate its contract.
But returning false still breaks hash tables and does not match the behaviour of the built in types, so I consider it a bad idea.

This can be avoided by calling Equals on the members instead of == in the implementation of Equals but not in == and !=.

For example with quaternion,

replace

public bool Equals(Quaternion other)
{
    return (X == other.X &&
            Y == other.Y &&
            Z == other.Z &&
            W == other.W);
}

with

public bool Equals(Quaternion other)
{
    return (X.Equals(other.X) &&
            Y.Equals(other.Y) &&
            Z.Equals(other.Z) &&
            W.Equals(other.W));
}

You might want to add tests that check that == and != compare all the above cases as unequal, so that they match the IEEE specification.

Replace:

// Counterintuitive result - IEEE rules for NaN comparison are weird!
Assert.False(a.Equals(a));
Assert.False(b.Equals(b));
Assert.False(c.Equals(c));
Assert.False(d.Equals(d));

with:

// Equals does not follow IEEE semantics since many consumers rely on equality being reflexive.
// This includes collections like `Dictionary<K,V>` or `HashSet<T>`
Assert.True(a.Equals(a));
Assert.True(b.Equals(b));
Assert.True(c.Equals(c));
Assert.True(d.Equals(d));

// Counterintuitive result - IEEE rules for NaN comparison are weird!
Assert.False(a == a);
Assert.False(b == b);
Assert.False(c == c);
Assert.False(d == d);

Assert.True(a != a);
Assert.True(b != b);
Assert.True(c != c);
Assert.True(d != d);

[mellinoe]

We can have a discussion about whether we want to change this behavior, but currently there's a couple more points to consider:

• On Vector 2/3/4, the Equals(Vector) method is recognized as a JIT intrinsic, meaning that the appropriate SIMD instructions are emitted in order to parallelize the comparisons (I'd have to look at some references to remember which instructions are appropriate here...). Generally this means that the instructions will follow the IEEE standard with regards to NaN values. In the past, we've matched our C# implementation to that, simply because it would be strange to have different behavior if you enabled or disabled SIMD support.
• Quaternion, Plane, Matrix3x2 and Matrix4x4 do not yet have JIT support, but I can see that the same argument will apply here, so I'd rather not change the behavior if we know it will not match the hardware instructions we hope to use eventually.

[CodesInChaos]

In my experience Equals is mostly used by containers and search algorithms which rely on reflexivity. For these algorithms a performance gain is irrelevant if prevents them from working. Don't sacrifice correctness for performance.

This breaks all hash based sets and dictionaries, Contains, Find, IndexOf on various collections/LINQ, set based LINQ operations (Union, Except, etc.).

Code that does actual computations where IEEE semantic is acceptable usually works on concrete types and uses == / != (or more likely epsilon comparisons). You currently can't write high performance computations using generics since you need arithmetic operations for that, but these aren't available through interfaces/virtual methods. If you add a common interface for float/double that enables arithmetic, you can use the same interface to add a IeeeEquals method.

I don't know any precedent in the BCL where Equals follows IEEE instead of being reflexive. Counter examples include Single, Double, Tuple and System.Numerics.Complex.

because it would be strange to have different behavior if you enabled or disabled SIMD support.

I agree that the behaviour shouldn't depend on the availability of SIMD. My suggestion is to be reflexive in Equals and follow IEEE for ==/!= regardless of SIMD availability even if that could reduce performance in some cases.

[jods4]

@CodesInChaos basically you say that you use equality on double or float and use them as hashtable keys or Find or IndexOf? This is a very dangerous proposition. Unless your floating point numbers have been carefully constructed you can't expect equality to work reliably even for non-NaN numbers.

I think using Equals on floating point numbers shouldn't be a supported scenario at all. That said @CodesInChaos has a point about consistent behavior with existing types.

A workarounds exists as it is possible to write a IEqualityComparer that does what you want.

[mellinoe]

I apologize for leaving this issue open for so long without giving any closure. For a couple of reasons, we won't be changing this behavior:

• As Equals() is recognized as a JIT intrinsic, switching it's behavior would essentially prevent us from utilizing any hardware speedup here. There may be some trickery we could do here to still get some hardware acceleration (the JIT team would know more about whether something like that is feasible), but using direct SIMD instructions for equality (which follow IEEE equality semantics) will be faster. Given that this is a performance-centric library, I don't think the tradeoff here would be worth it.
• This is a non-obvious behavior, in my opinion. Although the behavior is present in some of our primitive structs, I don't think it is necessarily an expected one. Existing vector libraries do not generally exhibit this type of equality behavior, in my experience, so I also don't think there is an expectation in the domain.