addressed review points

apache · Aug 31, 2023 · 6d5381e · 6d5381e
1 parent 216e5dc
commit 6d5381e
Show file tree

Hide file tree

Showing 5 changed files with 8 additions and 4 deletions.
diff --git a/fi/test/frequent_items_sketch_serialize_for_java.cpp b/fi/test/frequent_items_sketch_serialize_for_java.cpp
@@ -47,7 +47,7 @@ TEST_CASE("frequent strings sketch generate", "[serialize_for_java]") {
     for (unsigned i = 1; i <= n; ++i) sketch.update(std::to_string(i));
     REQUIRE(sketch.is_empty() == (n == 0));
     if (n > 10) {
-//      REQUIRE(sketch.get_maximum_error() > 0);
+      REQUIRE(sketch.get_maximum_error() > 0);
     } else {
       REQUIRE(sketch.get_maximum_error() == 0);
     }

diff --git a/sampling/test/var_opt_sketch_serialize_for_java.cpp b/sampling/test/var_opt_sketch_serialize_for_java.cpp
@@ -43,7 +43,7 @@ TEST_CASE("varopt sketch string exact", "[serialize_for_java]") {
 TEST_CASE("varopt sketch long sampling", "[serialize_for_java]") {
   var_opt_sketch<long> sketch(1024);
   for (unsigned i = 0; i < 2000; ++i) sketch.update(i);
-  // heavy items have negative weights to allow a simple predicate to filter
+  // negative heavy items to allow a simple predicate to filter
   sketch.update(-1L, 100000.0);
   sketch.update(-2L, 110000.0);
   sketch.update(-3L, 120000.0);

diff --git a/sampling/test/var_opt_union_serialize_for_java.cpp b/sampling/test/var_opt_union_serialize_for_java.cpp
@@ -23,7 +23,7 @@
 
 namespace datasketches {
 
-TEST_CASE("var opt union doulbe sampling", "[serialize_for_java]") {
+TEST_CASE("var opt union double sampling", "[serialize_for_java]") {
   const unsigned k_small = 16;
   const unsigned k_max = 128;
   const unsigned n1 = 32;
@@ -32,7 +32,7 @@ TEST_CASE("var opt union doulbe sampling", "[serialize_for_java]") {
   // small k sketch, but sampling
   var_opt_sketch<double> sketch1(k_small);
   for (unsigned i = 0; i < n1; ++i) sketch1.update(i);
-  // heavy items have negative weights to allow a simple predicate to filter
+  // negative heavy item to allow a simple predicate to filter
   sketch1.update(-1, n1 * n1);
 
   // another one, but different n to get a different per-item weight

diff --git a/theta/test/theta_sketch_serialize_for_java.cpp b/theta/test/theta_sketch_serialize_for_java.cpp
@@ -49,6 +49,8 @@ TEST_CASE("theta sketch generate compressed", "[serialize_for_java]") {
 
 TEST_CASE("theta sketch generate non-empty no entries", "[serialize_for_java]") {
   auto sketch = update_theta_sketch::builder().set_p(0.01).build();
+  // here we rely on the fact that hash of 1 happens to be greater than 0.01 (when normalized)
+  // and therefore gets rejected
   sketch.update(1);
   REQUIRE_FALSE(sketch.is_empty());
   REQUIRE(sketch.get_num_retained() == 0);

diff --git a/tuple/test/aod_sketch_serialize_for_java.cpp b/tuple/test/aod_sketch_serialize_for_java.cpp
@@ -49,6 +49,8 @@ TEST_CASE("aod sketch generate three values", "[serialize_for_java]") {
 
 TEST_CASE("aod sketch generate non-empty no entries", "[serialize_for_java]") {
   auto sketch = update_array_of_doubles_sketch::builder().set_p(0.01).build();
+  // here we rely on the fact that hash of 1 happens to be greater than 0.01 (when normalized)
+  // and therefore gets rejected
   sketch.update(1, std::vector<double>({1}));
   REQUIRE_FALSE(sketch.is_empty());
   REQUIRE(sketch.get_num_retained() == 0);