This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit b15d6dc2e7df05392a1daa4bc1b3da9ca31a583b Author: Michael Smith <[email protected]> AuthorDate: Tue Oct 10 18:32:25 2023 +0000 IMPALA-12490: Fix tests on ARM Fixes failing llvm-codegen-cache-test on ARM by increasing the expected cache entry size to account for different LLVM structure size. Updates compare_float to use an implementation matching isclose from PEP 485 (added in Python 3.5). The prior implementation only did absolute comparison, which works increasingly poorly for values greater than 1. Several cases in test_aggregation.py failed on ARM being off-by-one in the least significant digit but greater than the 1e-9 absolute epsilon. The prior link is also considered outdated, now redirecting to https://randomascii.wordpress.com/2012/02/25/comparing-floating-point-numbers-2012-edition/ which argues for similar treatment to PEP 485. Retains the same absolute tolerance for double and float to avoid changing behavior for very small numbers. Change-Id: Id60bd16402c09b9c939edc93a745c07f235b8912 Reviewed-on: http://gerrit.cloudera.org:8080/20557 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Michael Smith <[email protected]> --- be/src/codegen/llvm-codegen-cache-test.cc | 10 ++++++++-- tests/common/test_result_verifier.py | 11 ++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/be/src/codegen/llvm-codegen-cache-test.cc b/be/src/codegen/llvm-codegen-cache-test.cc index a07c51dda..70d419c70 100644 --- a/be/src/codegen/llvm-codegen-cache-test.cc +++ b/be/src/codegen/llvm-codegen-cache-test.cc @@ -269,8 +269,14 @@ int64_t LlvmCodeGenCacheTest::GetMemCharge( void LlvmCodeGenCacheTest::TestAtCapacity(TCodeGenCacheMode::type mode) { int64_t codegen_cache_capacity = 196; // 196B bool is_normal_mode = !CodeGenCacheModeAnalyzer::is_optimal(mode); - // 128B for optimal mode - if (!is_normal_mode) codegen_cache_capacity = 128; + // 128B for optimal mode, or 140B on ARM systems + if (!is_normal_mode) { +#ifdef __aarch64__ + codegen_cache_capacity = 140; +#else + codegen_cache_capacity = 128; +#endif + } // Using single shard makes the logic of scenarios simple for capacity and // eviction-related behavior. FLAGS_cache_force_single_shard = true; diff --git a/tests/common/test_result_verifier.py b/tests/common/test_result_verifier.py index d71fe6349..84e324744 100644 --- a/tests/common/test_result_verifier.py +++ b/tests/common/test_result_verifier.py @@ -185,16 +185,17 @@ def try_compile_regex(row_string): return regex return None + # If comparing against a float or double, don't do a strict comparison -# See: http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm -def compare_float(x, y, epsilon): +# See: https://peps.python.org/pep-0485/#proposed-implementation +def compare_float(x, y, rel_tol=1e-9, abs_tol=0.0): # For the purposes of test validation, we want to treat nans as equal. The # floating point spec defines nan != nan. if math.isnan(x) and math.isnan(y): return True if math.isinf(x) or math.isinf(y): return x == y - return abs(x - y) <= epsilon + return abs(x - y) <= max(rel_tol * max(abs(x), abs(y)), abs_tol) # Represents a column in a row class ResultColumn(object): @@ -227,9 +228,9 @@ class ResultColumn(object): if (self.value == 'NULL' or other.value == 'NULL'): return self.value == other.value elif self.column_type == 'float': - return compare_float(float(self.value), float(other.value), 10e-5) + return compare_float(float(self.value), float(other.value), abs_tol=10e-5) elif self.column_type == 'double': - return compare_float(float(self.value), float(other.value), 10e-10) + return compare_float(float(self.value), float(other.value), abs_tol=10e-10) elif self.column_type == 'boolean': return str(self.value).lower() == str(other.value).lower() else:
