Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 4.3k
GH-46395: [C++][Statistics] Use EqualOptions for min and max in arrow::ArrayStatistics::Equals()#46422
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
Merged
Uh oh!
There was an error while loading. Please reload this page.
Merged
GH-46395: [C++][Statistics] Use EqualOptions for min and max in arrow::ArrayStatistics::Equals() #46422
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
3374ac3
add Approximate equal
andishgar 168af2c
use correct format for double values
andishgar cd47933
remove ApproximateEquals method
andishgar 3cc2f93
Add is_approximate
andishgar 5560be3
Correct comments
andishgar 292d2af
Enable allow_atol in Equal options and add operator == and !=
andishgar 8d26a40
Apply Kou suggestion
andishgar fc5697c
Change the way that equality is implemented
andishgar ffde3c5
Apply Kou suggestion
andishgar 2a0760f
fix space, apply unified naming for fixture,
andishgar e06599f
fix minor linting
andishgar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -15,29 +15,33 @@ | ||
| // specific language governing permissions and limitations | ||
| // under the License. | ||
| #include <limits> | ||
| #include <variant> | ||
| #include <gtest/gtest.h> | ||
| #include "arrow/array/statistics.h" | ||
| #include "arrow/compare.h" | ||
andishgar marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| namespace arrow { | ||
| TEST(ArrayStatisticsTest, TestNullCount) { | ||
| TEST(TestArrayStatistics, NullCount) { | ||
| ArrayStatistics statistics; | ||
| ASSERT_FALSE(statistics.null_count.has_value()); | ||
| statistics.null_count = 29; | ||
| ASSERT_TRUE(statistics.null_count.has_value()); | ||
| ASSERT_EQ(29, statistics.null_count.value()); | ||
| } | ||
| TEST(ArrayStatisticsTest, TestDistinctCount) { | ||
| TEST(TestArrayStatistics, DistinctCount) { | ||
| ArrayStatistics statistics; | ||
| ASSERT_FALSE(statistics.distinct_count.has_value()); | ||
| statistics.distinct_count = 29; | ||
| ASSERT_TRUE(statistics.distinct_count.has_value()); | ||
| ASSERT_EQ(29, statistics.distinct_count.value()); | ||
| } | ||
| TEST(ArrayStatisticsTest, TestMin) { | ||
| TEST(TestArrayStatistics, Min) { | ||
| ArrayStatistics statistics; | ||
| ASSERT_FALSE(statistics.min.has_value()); | ||
| ASSERT_FALSE(statistics.is_min_exact); | ||
| @@ -49,7 +53,7 @@ TEST(ArrayStatisticsTest, TestMin) { | ||
| ASSERT_TRUE(statistics.is_min_exact); | ||
| } | ||
| TEST(ArrayStatisticsTest, TestMax) { | ||
| TEST(TestArrayStatistics, Max) { | ||
| ArrayStatistics statistics; | ||
| ASSERT_FALSE(statistics.max.has_value()); | ||
| ASSERT_FALSE(statistics.is_max_exact); | ||
| @@ -61,7 +65,7 @@ TEST(ArrayStatisticsTest, TestMax) { | ||
| ASSERT_FALSE(statistics.is_max_exact); | ||
| } | ||
| TEST(ArrayStatisticsTest, TestEquality) { | ||
| TEST(TestArrayStatistics, EqualityNonDoulbeValue) { | ||
| ArrayStatistics statistics1; | ||
| ArrayStatistics statistics2; | ||
| @@ -96,6 +100,56 @@ TEST(ArrayStatisticsTest, TestEquality) { | ||
| ASSERT_NE(statistics1, statistics2); | ||
| statistics2.is_max_exact = true; | ||
| ASSERT_EQ(statistics1, statistics2); | ||
| // Test different ArrayStatistics::ValueType | ||
| statistics1.max = static_cast<uint64_t>(29); | ||
| statistics1.max = static_cast<int64_t>(29); | ||
| ASSERT_NE(statistics1, statistics2); | ||
| } | ||
| class TestArrayStatisticsEqualityDoubleValue : public ::testing::Test { | ||
| protected: | ||
| ArrayStatistics statistics1_; | ||
| ArrayStatistics statistics2_; | ||
| EqualOptions options_ = EqualOptions::Defaults(); | ||
| }; | ||
| TEST_F(TestArrayStatisticsEqualityDoubleValue, ExactValue) { | ||
| statistics2_.min = 29.0; | ||
| statistics1_.min = 29.0; | ||
| ASSERT_EQ(statistics1_, statistics2_); | ||
| statistics2_.min = 30.0; | ||
| ASSERT_NE(statistics1_, statistics2_); | ||
| } | ||
| TEST_F(TestArrayStatisticsEqualityDoubleValue, SignedZero) { | ||
| statistics1_.min = +0.0; | ||
| statistics2_.min = -0.0; | ||
| ASSERT_TRUE(statistics1_.Equals(statistics2_, options_.signed_zeros_equal(true))); | ||
| ASSERT_FALSE(statistics1_.Equals(statistics2_, options_.signed_zeros_equal(false))); | ||
| } | ||
| TEST_F(TestArrayStatisticsEqualityDoubleValue, Infinity) { | ||
| auto infinity = std::numeric_limits<double>::infinity(); | ||
| statistics1_.min = infinity; | ||
| statistics2_.min = infinity; | ||
| ASSERT_EQ(statistics1_, statistics2_); | ||
| statistics1_.min = -infinity; | ||
| ASSERT_NE(statistics1_, statistics2_); | ||
| } | ||
| TEST_F(TestArrayStatisticsEqualityDoubleValue, NaN) { | ||
| statistics1_.min = std::numeric_limits<double>::quiet_NaN(); | ||
| statistics2_.min = std::numeric_limits<double>::quiet_NaN(); | ||
| ASSERT_TRUE(statistics1_.Equals(statistics2_, options_.nans_equal(true))); | ||
| ASSERT_FALSE(statistics1_.Equals(statistics2_, options_.nans_equal(false))); | ||
| } | ||
| TEST_F(TestArrayStatisticsEqualityDoubleValue, ApproximateEquals) { | ||
| statistics1_.max = 0.5001f; | ||
| statistics2_.max = 0.5; | ||
| ASSERT_FALSE(statistics1_.Equals(statistics2_, options_.atol(1e-3).use_atol(false))); | ||
| ASSERT_TRUE(statistics1_.Equals(statistics2_, options_.atol(1e-3))); | ||
| } | ||
| } // namespace arrow | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -27,6 +27,7 @@ | ||
| namespace arrow { | ||
| struct ArrayStatistics; | ||
| class Array; | ||
| class DataType; | ||
| class Tensor; | ||
| @@ -58,7 +59,18 @@ class EqualOptions { | ||
| return res; | ||
| } | ||
| /// Whether the "atol" property is used in the comparison. | ||
andishgar marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| bool use_atol() const { return use_atol_; } | ||
| /// Return a new EqualOptions object with the "use_atol" property changed. | ||
| EqualOptions use_atol(bool v) const { | ||
| auto res = EqualOptions(*this); | ||
| res.use_atol_ = v; | ||
| return res; | ||
| } | ||
| /// The absolute tolerance for approximate comparisons of floating-point values. | ||
| /// Note that this option is ignored if "use_atol" is set to false. | ||
| double atol() const { return atol_; } | ||
| /// Return a new EqualOptions object with the "atol" property changed. | ||
| @@ -87,6 +99,7 @@ class EqualOptions { | ||
| double atol_ = kDefaultAbsoluteTolerance; | ||
| bool nans_equal_ = false; | ||
| bool signed_zeros_equal_ = true; | ||
| bool use_atol_ = true; | ||
andishgar marked this conversation as resolved.
Outdated
Uh oh!There was an error while loading. Please reload this page. | ||
| std::ostream* diff_sink_ = NULLPTR; | ||
| }; | ||
| @@ -135,6 +148,16 @@ ARROW_EXPORT bool SparseTensorEquals(const SparseTensor& left, const SparseTenso | ||
| ARROW_EXPORT bool TypeEquals(const DataType& left, const DataType& right, | ||
| bool check_metadata = true); | ||
| /// \brief Check two \ref arrow::ArrayStatistics for equality | ||
| /// \param[in] left an \ref arrow::ArrayStatistics | ||
| /// \param[in] right an \ref arrow::ArrayStatistics | ||
| /// \param[in] options Options used to compare double values for equality. | ||
| /// \return True if the two \ref arrow::ArrayStatistics instances are equal; otherwise, | ||
| /// false. | ||
| ARROW_EXPORT bool ArrayStatisticsEquals( | ||
| const ArrayStatistics& left, const ArrayStatistics& right, | ||
| const EqualOptions& options = EqualOptions::Defaults()); | ||
| /// Returns true if scalars are equal | ||
| /// \param[in] left a Scalar | ||
| /// \param[in] right a Scalar | ||
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.