[C++] Incorrect Float16 Comparison for NaN and signed zero

[andishgar]

Describe the bug, including details regarding any error messages, version, and platform.

In the code below, two NaN values are considered equal regardless of whetherEqualOptions::nan_equalis set to true or false. The root of the problem lies here, where float16 values with the same memory representation are not correctly handled, and also here, where HalfFloat is implemented as a uint16_t, causing the comparison to return true.

TEST(FloatComparasion, Float16NaN) {
  auto half_float_array = ArrayFromJSON(float16(), R"([16.0,NaN])");
  auto half_float_array_1 = ArrayFromJSON(float16(), R"([16.0,NaN])");
  ASSERT_TRUE(half_float_array->Equals(half_float_array_1, EqualOptions::Defaults().nans_equal(false)));
  ASSERT_TRUE(half_float_array->Equals(half_float_array, EqualOptions::Defaults().nans_equal(false)));
}

Component(s)

C++

[andishgar]

@kou Could you verify my observation?

[kou]

It seems that your observation is correct.

[shreyavarma99]

I'd like to take this one. I'll send a PR soon!

[andishgar]

@shreyavarma99 Thank you for addressing this issue. I forgot to mention an additional note (sorry for that). The same problem occurs with signed zero as well.

TEST(FloatComparasion, float16) {

  auto f1 = util::Float16::FromDouble(+0.0);
  auto f2 = util::Float16::FromDouble(-0.0);
  ASSERT_TRUE(f1.is_zero());
  ASSERT_TRUE(f2.is_zero());
  ASSERT_TRUE(f2.signbit());
  HalfFloatScalar scalar_1(f1.bits());
  HalfFloatScalar scalar_2(f2.bits());

  f1 = util::Float16::FromBits(scalar_1.value);
  f2 = util::Float16::FromBits(scalar_2.value);
  ASSERT_TRUE(f1.is_zero());
  ASSERT_TRUE(f2.is_zero());
  ASSERT_TRUE(f2.signbit());
  ASSERT_FALSE(
      scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(true)));
  ASSERT_FALSE(
      scalar_1.Equals(scalar_2, EqualOptions::Defaults().signed_zeros_equal(false)));
}

[pitrou]

Issue resolved by pull request 46973
#46973