andishgar commented on issue #48123: URL: https://github.com/apache/arrow/issues/48123#issuecomment-3537362579
@pitrou, sorry for my delay in answering this. 1. [The helper function ](https://github.com/apache/arrow/blob/0c2087452d138ed51f23d0dc7a5baff7bab87849/cpp/src/arrow/testing/math.h#L24-L32)has a bug, and here is the code that proves it. ```c++ FloatUnion float_union(0.0); float_union.value_ += 1; float v1 = float_union.f_; float_union.value_ = float_union.value_ | (1 << 31); float v2 = float_union.f_; ASSERT_EQ(v1, std::numeric_limits<float>::denorm_min()); ASSERT_EQ(v2, -1 * std::numeric_limits<float>::denorm_min()); ASSERT_FLOAT_EQ(v1, v2); auto a = std::numeric_limits<float>::denorm_min(); auto b = -1 * a; auto c = std::nextafterf(b, 2.0); c = std::nextafterf(c, 3.0); ASSERT_EQ(a, c); // Although variable `b` reaches the value of variable `a` after calling // `std::nextafterf` twice, the following incorrect assertion still passes. ASSERT_FALSE(WithinUlp(a, b, 2)); ``` 2. I want to modify the implementation suggested in this [blog](https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/)(Under the section of **ULP , he said nervously**), which has already been implemented in [LLVM](https://github.com/llvm/llvm-project/blob/5ed26ade0c585d2a081f23546902f64a0366de19/libc/src/__support/FPUtil/ManipulationFunctions.h#L213-L245) (for `nextafter`) and in [GTest](https://github.com/google/googletest/blob/085af2cc08600bdb13827ca40261abcbe5048bb5/googletest/include/gtest/internal/gtest-internal.h#L372-L376) (for `ASSERT_DOUBLE_EQ`). The only problem with the blog is that it does not mention how to handle numbers with different signs(Even considering it is impractical), whereas GTest provides a [solution](https://github.com/google/googletest/blob/085af2cc08600bdb13827ca40261abcbe5048bb5/googletest/include/gtest/internal/gtest-internal.h#L360-L368) for that. What is your opinion? Can I update the Arrow implementation to u se this new solution? 3. What is your opinion on enabling this method of comparison in the Arrow main module for equality of `Array`, `Scalar`, `RecordBatch`, and `Table` as an optional feature? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
