Merge pull request #495 from proost/fix-allow-positive-weight-only

fix: allow positive weight only

Author: leerho

sampling/include/var_opt_sketch.hpp

@@ -272,7 +272,7 @@ class var_opt_sketch {
     typedef typename std::allocator_traits<A>::template rebind_alloc<double> AllocDouble;
     typedef typename std::allocator_traits<A>::template rebind_alloc<bool> AllocBool;
 
-    static const uint32_t MIN_LG_ARR_ITEMS = 3;
+    static const uint32_t MIN_LG_ARR_ITEMS = 4;
 
     static const uint8_t PREAMBLE_LONGS_EMPTY  = 1;
     static const uint8_t PREAMBLE_LONGS_WARMUP = 3;

sampling/include/var_opt_sketch_impl.hpp

@@ -772,12 +772,11 @@ string<A> var_opt_sketch<T, A>::items_to_string(bool print_gap) const {
 template<typename T, typename A>
 template<typename O>
 void var_opt_sketch<T, A>::update(O&& item, double weight, bool mark) {
-  if (weight < 0.0 || std::isnan(weight) || std::isinf(weight)) {
-    throw std::invalid_argument("Item weights must be nonnegative and finite. Found: "
+  if (weight <= 0.0 || std::isnan(weight) || std::isinf(weight)) {
+    throw std::invalid_argument("Item weights must be positive and finite. Found: "
                                 + std::to_string(weight));
-  } else if (weight == 0.0) {
-    return;
   }
+
   ++n_;
 
   if (r_ == 0) {

sampling/test/var_opt_sketch_test.cpp

@@ -178,11 +178,17 @@ TEST_CASE("varopt sketch: non-empty degenerate sketch", "[var_opt_sketch]") {
 
 TEST_CASE("varopt sketch: invalid weight", "[var_opt_sketch]") {
   var_opt_sketch<std::string> sk(100, resize_factor::X2);
-  REQUIRE_THROWS_AS(sk.update("invalid_weight", -1.0), std::invalid_argument);
 
-  // should not throw but sketch should still be empty
-  sk.update("zero weight", 0.0);
-  REQUIRE(sk.is_empty());
+  // Negative
+  REQUIRE_THROWS_AS(sk.update("invalid_weight", -1.0), std::invalid_argument);
+  // Zero
+  REQUIRE_THROWS_AS(sk.update("zero_weight", 0.0), std::invalid_argument);
+  // NaN
+  REQUIRE_THROWS_AS(sk.update("NaN_weight", std::numeric_limits<double>::quiet_NaN()), std::invalid_argument);
+  // +Inf
+  REQUIRE_THROWS_AS(sk.update("positive_infinity", std::numeric_limits<double>::infinity()), std::invalid_argument);
+  // -Inf
+  REQUIRE_THROWS_AS(sk.update("negative_infinity", -std::numeric_limits<double>::infinity()), std::invalid_argument);
 }
 
 TEST_CASE("varopt sketch: corrupt serialized weight", "[var_opt_sketch]") {