-
Notifications
You must be signed in to change notification settings - Fork 4k
GH-48408: [C++] Enable ULP-based float comparison #49290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,6 +35,7 @@ class SparseTensor; | |
| struct Scalar; | ||
|
|
||
| static constexpr double kDefaultAbsoluteTolerance = 1E-5; | ||
| static constexpr uint16_t kDefaultUlpDistance = 4; | ||
|
|
||
| /// A container of options for equality comparisons | ||
| class EqualOptions { | ||
|
|
@@ -66,6 +67,8 @@ class EqualOptions { | |
| bool use_atol() const { return use_atol_; } | ||
|
|
||
| /// Return a new EqualOptions object with the "use_atol" property changed. | ||
| /// If both "ulp_distance" and "atol" are specified, the comparison | ||
| /// succeeds when either condition is satisfied. | ||
| EqualOptions use_atol(bool v) const { | ||
| auto res = EqualOptions(*this); | ||
| res.use_atol_ = v; | ||
|
|
@@ -115,6 +118,31 @@ class EqualOptions { | |
| return res; | ||
| } | ||
|
|
||
| /// Whether the "ulp_distance" property is used in the comparison. | ||
| /// | ||
| /// This option only affects the Equals methods | ||
| /// and has no effect on ApproxEquals methods. | ||
| /// If both "ulp_distance" and "atol" are specified, the comparison | ||
| /// succeeds when either condition is satisfied. | ||
| bool use_ulp_distance() const { return use_ulp_distance_; } | ||
|
|
||
| /// Return a new EqualOptions object with the "use_ulp_distance" property changed. | ||
| EqualOptions use_ulp_distance(bool v) const { | ||
| auto res = EqualOptions(*this); | ||
| res.use_ulp_distance_ = v; | ||
| return res; | ||
| } | ||
|
|
||
| /// The ulp distance for approximate comparisons of floating-point values. | ||
| /// Note that this option is ignored if "use_ulp_distance" is set to false. | ||
| uint16_t ulp_distance() const { return ulp_distance_; } | ||
|
|
||
| /// Return a new EqualOptions object with the "ulp_distance" property changed. | ||
| EqualOptions ulp_distance(uint16_t v) { | ||
| auto res = EqualOptions(*this); | ||
| res.ulp_distance_ = v; | ||
| return res; | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is starting to be a lot of methods just to customize FP comparison. Also, usually you know the number of ULPs you want, so you have to call two methods ( I wonder if we could have a single method to encompass all these usages, for example: auto options = EqualOptions().float_comparison(EqualOptions::Atol(1e-5));
auto options2 = EqualOptions().float_comparison(EqualOptions::Ulps(2)); |
||
| /// The ostream to which a diff will be formatted if arrays disagree. | ||
| /// If this is null (the default) no diff will be formatted. | ||
| std::ostream* diff_sink() const { return diff_sink_; } | ||
|
|
@@ -132,11 +160,13 @@ class EqualOptions { | |
|
|
||
| protected: | ||
| double atol_ = kDefaultAbsoluteTolerance; | ||
| uint16_t ulp_distance_ = kDefaultUlpDistance; | ||
| bool nans_equal_ = false; | ||
| bool signed_zeros_equal_ = true; | ||
| bool use_atol_ = false; | ||
| bool use_schema_ = true; | ||
| bool use_metadata_ = false; | ||
| bool use_ulp_distance_ = false; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could have a enum to simplify those flags slightly, for example: enum FloatComparison { Exact, Atol, Ulps };
FloatComparison float_comparison_ = Exact;Alternatively, we could use a std::variant to also encompass the numeric parameters: struct ExactComparison {};
struct UlpComparison { int max_ulps; }
struct AtolComparison { double atol; }
// Defaults to ExactComparison
std::variant<ExactComparison, UlpComparison, AtolComparison> float_comparison_ = {}; |
||
|
|
||
| std::ostream* diff_sink_ = NULLPTR; | ||
| }; | ||
|
|
@@ -147,8 +177,8 @@ class EqualOptions { | |
| ARROW_EXPORT bool ArrayEquals(const Array& left, const Array& right, | ||
| const EqualOptions& = EqualOptions::Defaults()); | ||
|
|
||
| /// Returns true if the arrays are approximately equal. For non-floating point | ||
| /// types, this is equivalent to ArrayEquals(left, right) | ||
| /// Returns true if the arrays are approximately equal according to the absolute tolerance | ||
| /// method. For non-floating point types, this is equivalent to ArrayEquals(left, right) | ||
| /// | ||
| /// Note that arrow::ArrayStatistics is not included in the comparison. | ||
| ARROW_EXPORT bool ArrayApproxEquals(const Array& left, const Array& right, | ||
|
|
@@ -163,6 +193,7 @@ ARROW_EXPORT bool ArrayRangeEquals(const Array& left, const Array& right, | |
| const EqualOptions& = EqualOptions::Defaults()); | ||
|
|
||
| /// Returns true if indicated equal-length segment of arrays are approximately equal | ||
| /// according to the absolute tolerance method. | ||
| /// | ||
| /// Note that arrow::ArrayStatistics is not included in the comparison. | ||
| ARROW_EXPORT bool ArrayRangeApproxEquals(const Array& left, const Array& right, | ||
|
|
@@ -202,7 +233,8 @@ ARROW_EXPORT bool ArrayStatisticsEquals( | |
| ARROW_EXPORT bool ScalarEquals(const Scalar& left, const Scalar& right, | ||
| const EqualOptions& options = EqualOptions::Defaults()); | ||
|
|
||
| /// Returns true if scalars are approximately equal | ||
| /// Returns true if the scalars are approximately equal according to the absolute | ||
| /// tolerance method. | ||
| /// \param[in] left a Scalar | ||
| /// \param[in] right a Scalar | ||
| /// \param[in] options comparison options | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.