[Discussion][C++][Statistics] Should ApproximateEquals or Equals be used to handle floating-point precision errors in arrow::ArrayStatistics?

[andishgar]

This pull request includes an implementation to compare the double values of the max and min member variables of arrow::ArrayStatistics . The question, however, is whether we should use arrow::ArrayStatistics::Equal to handle floating-point precision errors or introduce a new method named arrow::ArrayStatistics::ApproximateEquals for this purpose.

Aside from that, there are several other statistical attributes with double values, although they have not yet been implemented in arrow::ArrayStatistics.

Component(s)

C++

[andishgar]

@kou, Could you share your opnion?

[kou]

My opinion:

• Add arrow::EqualOptions::floating_approximate or something
• Use arrow::ArrayStatistics::Equals() not arrow::ArrayStatistics::ApproximateEquals() for approximate equal too

arrow::Array::ApproximateEquals() was introduced by #219 and arrow::EqualOptions was introduced by #4005 . I feel that if we have arrow::EqualOptions when we added arrow::Array::ApproximateEquals(), we didn't add arrow::Array::ApproximateEquals().

@pitrou @bkietz @zanmato1984 @wgtmac @lidavidm Do you have any opinion for arrow::Array{,Statistics}::{,Aproximate}Equals() API?

[lidavidm]

I think using EqualOptions is preferable (e.g. you can specify tolerance, NaN behavior, etc.)

[andishgar]

I think using EqualOptions is preferable (e.g. you can specify tolerance, NaN behavior, etc.)

@lidavidm Thanks for sharing your opinion! Just a quick note: both Equals and ApproximateEquals accept EqualOptions. The main question is whether we should use ApproximateEquals or Equals for the following lines. (For reference, ApproximateEquals is currently used in the C++ implementation of arrow::Array for similar scenarios.)

https://github.com/apache/arrow/blob/afb535b7045ebdfdde1ba2fbb420ac3527a03f89/cpp/src/arrow/array/statistics.cc#L33-L35

[andishgar]

@pitrou @wgtmac, it seems you both agree with @lidavidm. Could you please share your thoughts on this comment

[kou]

Let's use the EqualOptions approach (no ApproximateEquals()).

[zanmato1984]

Sorry for that late response.

+1 to EqualOptions + regular ArrayStatistics::Equals().