pitrou commented on code in PR #48224:
URL: https://github.com/apache/arrow/pull/48224#discussion_r2587950878
##########
cpp/src/arrow/testing/math.cc:
##########
@@ -17,53 +17,107 @@
#include "arrow/testing/math.h"
+#include <algorithm>
#include <cmath>
#include <limits>
+#include <type_traits>
#include <gtest/gtest.h>
+#include "arrow/util/float16.h"
#include "arrow/util/logging_internal.h"
namespace arrow {
namespace {
template <typename Float>
-bool WithinUlpOneWay(Float left, Float right, int n_ulps) {
- // The delta between 1.0 and the FP value immediately before it.
- // We're using this value because `frexp` returns a mantissa between 0.5 and
1.0.
- static const Float kOneUlp = Float(1.0) - std::nextafter(Float(1.0),
Float(0.0));
+struct FloatToUInt;
- DCHECK_GE(n_ulps, 1);
+template <>
+struct FloatToUInt<double> {
+ using Type = uint64_t;
+};
- if (left == 0) {
- return left == right;
+template <>
+struct FloatToUInt<float> {
+ using Type = uint32_t;
+};
+
+template <>
+struct FloatToUInt<util::Float16> {
+ using Type = uint16_t;
+};
+
+template <typename Float>
+struct UlpDistanceUtil {
+ public:
+ using UIntType = typename FloatToUInt<Float>::Type;
+ static const UIntType kNumberOfBits = sizeof(Float) * 8;
+ static const UIntType kSignMask = static_cast<UIntType>(1) << (kNumberOfBits
- 1);
+
+ // This implementation is inspired by:
+ //
https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/
+ static UIntType UlpDistnace(Float left, Float right) {
+ auto unsigned_left = BitCast<UIntType>(left);
+ auto unsigned_right = BitCast<UIntType>(right);
+ auto biased_left = ConvertSignAndMagnitudeToBiased(unsigned_left);
+ auto biased_right = ConvertSignAndMagnitudeToBiased(unsigned_right);
+ if (biased_left > biased_right) {
+ std::swap(biased_left, biased_right);
+ }
+ return biased_right - biased_left;
}
- if (left < 0) {
- left = -left;
- right = -right;
+
+ private:
+ template <typename To, typename From>
+ union BitCastUnion {
+ explicit BitCastUnion(From from) : from(from) {}
+ From from;
+ To to;
+ };
+
+ template <typename To, typename From>
+ static UIntType BitCast(From value) {
+ assert(sizeof(To) == sizeof(From));
+ BitCastUnion<To, From> bit_cast(value);
+ return bit_cast.to;
}
Review Comment:
> 1-Is there any point in using this function? I mean, when the function is
called, the value has already been loaded. This function is supposed to load
the value byte by byte via memcpy, right?
In practice, the constant-size memcpy call will be optimized away by the
compiler.
> 2-This only works for types that have a default constructor. However, it
is possible for a type to be trivially copyable without having a default
constructor. Shouldn’t this be considered a bug?
I don't know, is this relevant to using it with floating-point numbers?
--
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]
