UnaryUfuncRealHBToFloatHTest: test Half more widely (#5676)

swolchok · facebook-github-bot · commit 07bcd7f15a9e · 2024-09-30T23:43:43.000-07:00
Summary: Pull Request resolved: pytorch/executorch#5676 The tests were trying to avoid Half in ATen mode, but that should work fine for these ops. ghstack-source-id: 245578279 Reviewed By: mergennachin Differential Revision: D63435866 fbshipit-source-id: 934b4453fa13df13619092fde979c06d51925663
diff --git a/kernels/test/UnaryUfuncRealHBToFloatHTest.cpp b/kernels/test/UnaryUfuncRealHBToFloatHTest.cpp
@@ -38,9 +38,6 @@ void UnaryUfuncRealHBToFloatHTest::test_mismatched_input_shapes_dies() {
 
 void UnaryUfuncRealHBToFloatHTest::
     test_all_real_input_half_output_static_dynamism_support() {
-  if (get_supported_features()->is_aten) {
-    GTEST_SKIP() << "Test Half support only for ExecuTorch mode";
-  }
 #define TEST_ENTRY(ctype, dtype)    \
   test_floating_point_op_out<       \
       exec_aten::ScalarType::dtype, \
@@ -55,7 +52,7 @@ void UnaryUfuncRealHBToFloatHTest::
   test_floating_point_op_out<       \
       exec_aten::ScalarType::dtype, \
       exec_aten::ScalarType::Float>();
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
@@ -65,15 +62,12 @@ void UnaryUfuncRealHBToFloatHTest::
   test_floating_point_op_out<       \
       exec_aten::ScalarType::dtype, \
       exec_aten::ScalarType::Double>();
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
 void UnaryUfuncRealHBToFloatHTest::
     test_all_real_input_half_output_bound_dynamism_support() {
-  if (get_supported_features()->is_aten) {
-    GTEST_SKIP() << "Test Half support only for ExecuTorch mode";
-  }
 #define TEST_ENTRY(ctype, dtype)    \
   test_floating_point_op_out<       \
       exec_aten::ScalarType::dtype, \
@@ -90,7 +84,7 @@ void UnaryUfuncRealHBToFloatHTest::
       exec_aten::ScalarType::dtype,  \
       exec_aten::ScalarType::Float>( \
       {10, 10}, exec_aten::TensorShapeDynamism::DYNAMIC_BOUND);
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
@@ -101,7 +95,7 @@ void UnaryUfuncRealHBToFloatHTest::
       exec_aten::ScalarType::dtype,   \
       exec_aten::ScalarType::Double>( \
       {10, 10}, exec_aten::TensorShapeDynamism::DYNAMIC_BOUND);
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
@@ -115,7 +109,7 @@ void UnaryUfuncRealHBToFloatHTest::
       exec_aten::ScalarType::dtype,  \
       exec_aten::ScalarType::Float>( \
       {1, 1}, exec_aten::TensorShapeDynamism::DYNAMIC_UNBOUND);
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
@@ -129,7 +123,7 @@ void UnaryUfuncRealHBToFloatHTest::
       exec_aten::ScalarType::dtype,   \
       exec_aten::ScalarType::Double>( \
       {1, 1}, exec_aten::TensorShapeDynamism::DYNAMIC_UNBOUND);
-  ET_FORALL_REAL_TYPES(TEST_ENTRY);
+  ET_FORALL_REALH_TYPES(TEST_ENTRY);
 #undef TEST_ENTRY
 }
 
diff --git a/kernels/test/UnaryUfuncRealHBToFloatHTest.h b/kernels/test/UnaryUfuncRealHBToFloatHTest.h
@@ -69,7 +69,17 @@ class UnaryUfuncRealHBToFloatHTest : public OperatorTest {
     op_out(tf_in.make({1, 6}, test_vector), out);
 
     auto expected = tf_out.make({1, 6}, expected_vector);
-    EXPECT_TENSOR_CLOSE(out, expected);
+    if (IN_DTYPE == ScalarType::Half || OUT_DTYPE == ScalarType::Half) {
+      double rtol = executorch::runtime::testing::internal::kDefaultRtol;
+      // It appears we need a higher tolerance for at least some ATen
+      // tests, like aten_op_acosh_test.
+      if (get_supported_features()->is_aten) {
+        rtol = 1e-3;
+      }
+      EXPECT_TENSOR_CLOSE_WITH_TOL(out, expected, rtol, executorch::runtime::testing::internal::kDefaultHalfAtol);
+    } else {
+      EXPECT_TENSOR_CLOSE(out, expected);
+    }
     // clang-format on
   }
 
diff --git a/runtime/core/exec_aten/testing_util/tensor_util.cpp b/runtime/core/exec_aten/testing_util/tensor_util.cpp
@@ -76,13 +76,19 @@ bool data_is_close(
   return true;
 }
 
+double default_atol_for_type(ScalarType t) {
+  if (t == ScalarType::Half) {
+    return internal::kDefaultHalfAtol;
+  }
+  return internal::kDefaultAtol;
+}
 } // namespace
 
 bool tensors_are_close(
     const Tensor& a,
     const Tensor& b,
     double rtol,
-    double atol) {
+    std::optional<double> opt_atol) {
   if (a.scalar_type() != b.scalar_type() || a.sizes() != b.sizes()) {
     return false;
   }
@@ -100,6 +106,8 @@ bool tensors_are_close(
   // So we can just compare the two underlying data sequentially to figure out
   // if the two tensors are same.
 
+  double atol = opt_atol.value_or(default_atol_for_type(a.scalar_type()));
+
   if (a.nbytes() == 0) {
     // Note that this case is important. It's valid for a zero-size tensor to
     // have a null data pointer, but in some environments it's invalid to pass a
@@ -149,11 +157,12 @@ bool tensor_data_is_close(
     const Tensor& a,
     const Tensor& b,
     double rtol,
-    double atol) {
+    std::optional<double> opt_atol) {
   if (a.scalar_type() != b.scalar_type() || a.numel() != b.numel()) {
     return false;
   }
 
+  double atol = opt_atol.value_or(default_atol_for_type(a.scalar_type()));
   if (a.nbytes() == 0) {
     // Note that this case is important. It's valid for a zero-size tensor to
     // have a null data pointer, but in some environments it's invalid to pass a
@@ -185,12 +194,12 @@ bool tensor_lists_are_close(
     const exec_aten::Tensor* tensors_b,
     size_t num_tensors_b,
     double rtol,
-    double atol) {
+    std::optional<double> opt_atol) {
   if (num_tensors_a != num_tensors_b) {
     return false;
   }
   for (size_t i = 0; i < num_tensors_a; i++) {
-    if (!tensors_are_close(tensors_a[i], tensors_b[i], rtol, atol)) {
+    if (!tensors_are_close(tensors_a[i], tensors_b[i], rtol, opt_atol)) {
       return false;
     }
   }
diff --git a/runtime/core/exec_aten/testing_util/tensor_util.h b/runtime/core/exec_aten/testing_util/tensor_util.h
@@ -11,13 +11,19 @@
 #include <executorch/runtime/core/exec_aten/exec_aten.h>
 #include <gmock/gmock.h> // For MATCHER_P
 
+#include <optional>
+
 namespace executorch {
 namespace runtime {
 namespace testing {
 
 namespace internal {
 constexpr double kDefaultRtol = 1e-5;
 constexpr double kDefaultAtol = 1e-8;
+// Per
+// https://en.wikipedia.org/wiki/Half-precision_floating-point_format,
+// float16 has about 3.3 digits of precision.
+constexpr double kDefaultHalfAtol = 1e-3;
 } // namespace internal
 
 /**
@@ -61,7 +67,7 @@ bool tensors_are_close(
     const exec_aten::Tensor& a,
     const exec_aten::Tensor& b,
     double rtol = internal::kDefaultRtol,
-    double atol = internal::kDefaultAtol);
+    std::optional<double> opt_atol = std::nullopt);
 
 /**
  * Returns true if the tensors are of the same numel and dtype, and if all
@@ -92,7 +98,7 @@ bool tensor_data_is_close(
     const exec_aten::Tensor& a,
     const exec_aten::Tensor& b,
     double rtol = internal::kDefaultRtol,
-    double atol = internal::kDefaultAtol);
+    std::optional<double> opt_atol = std::nullopt);
 
 /**
  * Returns true if the two lists are of the same length, and
@@ -105,7 +111,7 @@ bool tensor_lists_are_close(
     const exec_aten::Tensor* tensors_b,
     size_t num_tensors_b,
     double rtol = internal::kDefaultRtol,
-    double atol = internal::kDefaultAtol);
+    std::optional<double> opt_atol = std::nullopt);
 
 /**
  * Lets gtest users write `EXPECT_THAT(tensor1, IsCloseTo(tensor2))` or
