Return the exact min and max at ranks 0 and 1 in KLL, quantiles and REQ sketches

common/include/quantiles_sorted_view.hpp

@@ -28,7 +28,9 @@
 namespace datasketches {
 
 /**
- * Sorted view for quantiles sketches (REQ, KLL and Quantiles)
+ * Sorted view for quantiles sketches (REQ, KLL and Quantiles).
+ * For non-arithmetic types, the view refers to the items of the sketch it came from,
+ * so it must not be used after that sketch is updated, merged, assigned to, moved or destroyed.
  */
 template<
   typename T,
@@ -52,6 +54,9 @@ class quantiles_sorted_view {
   /// @private
   void convert_to_cummulative();
 
+  /// @private
+  void include_min_max(const T& min_item, const T& max_item);
+
   class const_iterator;
 
   /**

common/include/quantiles_sorted_view_impl.hpp

@@ -60,6 +60,20 @@ void quantiles_sorted_view<T, C, A>::convert_to_cummulative() {
   }
 }
 
+// Compaction can drop the min and max, but the sketch stores them exactly. Add each back as an entry
+// of weight 1, taken from its neighbor, so ranks 0 and 1 return them (IncludeMinMax in Java).
+template<typename T, typename C, typename A>
+void quantiles_sorted_view<T, C, A>::include_min_max(const T& min_item, const T& max_item) {
+  const bool add_min = comparator_(min_item, deref_helper(entries_.front().first));
+  const bool add_max = comparator_(deref_helper(entries_.back().first), max_item);
+  entries_.reserve(entries_.size() + (add_min ? 1 : 0) + (add_max ? 1 : 0));
+  if (add_max) {
+    entries_.back().second = total_weight_ - 1;
+    entries_.push_back(Entry(ref_helper(max_item), total_weight_));
+  }
+  if (add_min) entries_.insert(entries_.begin(), Entry(ref_helper(min_item), 1));
+}
+
 template<typename T, typename C, typename A>
 double quantiles_sorted_view<T, C, A>::get_rank(const T& item, bool inclusive) const {
   if (entries_.empty()) throw std::runtime_error("operation is undefined for an empty sketch");

common/test/quantiles_sorted_view_test.cpp

@@ -456,4 +456,70 @@ TEST_CASE("set 4", "sorted view") {
     REQUIRE(view.get_quantile(1, false) == 40);
 }
 
+TEST_CASE("include min and max", "sorted view") {
+  auto view = quantiles_sorted_view<float, std::less<float>, std::allocator<float>>(4, std::less<float>(), std::allocator<float>());
+  std::vector<float> l1 {2, 4, 6, 7};
+  view.add(l1.begin(), l1.end(), 2);
+  view.convert_to_cummulative();
+  view.include_min_max(1, 8);
+  REQUIRE(view.size() == 6);
+
+  const std::vector<std::pair<float, uint64_t>> expected {{1, 1}, {2, 2}, {4, 4}, {6, 6}, {7, 7}, {8, 8}};
+  auto it = view.begin();
+  for (const auto& entry: expected) {
+    REQUIRE(it->first == entry.first);
+    REQUIRE(it->second == entry.second);
+    ++it;
+  }
+  REQUIRE(it == view.end());
+
+  REQUIRE(view.get_quantile(0, true) == 1);
+  REQUIRE(view.get_quantile(0, false) == 1);
+  REQUIRE(view.get_quantile(1, true) == 8);
+  REQUIRE(view.get_quantile(1, false) == 8);
+
+  REQUIRE(view.get_rank(1, true) == 0.125);
+  REQUIRE(view.get_rank(8, false) == 0.875);
+}
+
+TEST_CASE("include min and max already retained", "sorted view") {
+  auto view = quantiles_sorted_view<float, std::less<float>, std::allocator<float>>(4, std::less<float>(), std::allocator<float>());
+  std::vector<float> l1 {2, 4, 6, 7};
+  view.add(l1.begin(), l1.end(), 2);
+  view.convert_to_cummulative();
+  view.include_min_max(2, 7);
+  REQUIRE(view.size() == 4);
+
+  const std::vector<std::pair<float, uint64_t>> expected {{2, 2}, {4, 4}, {6, 6}, {7, 8}};
+  auto it = view.begin();
+  for (const auto& entry: expected) {
+    REQUIRE(it->first == entry.first);
+    REQUIRE(it->second == entry.second);
+    ++it;
+  }
+  REQUIRE(it == view.end());
+}
+
+TEST_CASE("include min and max with one retained item", "sorted view") {
+  auto view = quantiles_sorted_view<float, std::less<float>, std::allocator<float>>(1, std::less<float>(), std::allocator<float>());
+  std::vector<float> l2 {5};
+  view.add(l2.begin(), l2.end(), 4);
+  view.convert_to_cummulative();
+  view.include_min_max(1, 9);
+  REQUIRE(view.size() == 3);
+
+  const std::vector<std::pair<float, uint64_t>> expected {{1, 1}, {5, 3}, {9, 4}};
+  auto it = view.begin();
+  for (const auto& entry: expected) {
+    REQUIRE(it->first == entry.first);
+    REQUIRE(it->second == entry.second);
+    ++it;
+  }
+  REQUIRE(it == view.end());
+
+  REQUIRE(view.get_quantile(0) == 1);
+  REQUIRE(view.get_quantile(0.5) == 5);
+  REQUIRE(view.get_quantile(1) == 9);
+}
+
 } /* namespace datasketches */

kll/include/kll_sketch.hpp

@@ -308,6 +308,9 @@ class kll_sketch {
      *
      * <p>If the sketch is empty this throws std::runtime_error.
      *
+     * <p>For non-arithmetic types, the returned reference refers to an item of this sketch, so it
+     * must not be used after this sketch is updated, merged, assigned to, moved or destroyed.
+     *
      * @param rank of an item in the hypothetical sorted stream.
      * @param inclusive if true, the given rank is considered inclusive (includes weight of an item)
      *
@@ -520,7 +523,9 @@ class kll_sketch {
     const_iterator end() const;
 
     /**
-     * Gets the sorted view of this sketch
+     * Gets the sorted view of this sketch.
+     * For non-arithmetic types, the view refers to the items of this sketch, so it must not be used
+     * after this sketch is updated, merged, assigned to, moved or destroyed.
      * @return the sorted view of this sketch
      */
     quantiles_sorted_view<T, C, A> get_sorted_view() const;

kll/include/kll_sketch_impl.hpp

@@ -775,6 +775,7 @@ quantiles_sorted_view<T, C, A> kll_sketch<T, C, A>::get_sorted_view() const {
     view.add(from, to, 1ULL << level);
   }
   view.convert_to_cummulative();
+  if (!is_empty()) view.include_min_max(*min_item_, *max_item_);
   return view;
 }
 

kll/test/kll_sketch_test.cpp

@@ -202,6 +202,96 @@ TEST_CASE("kll sketch", "[kll_sketch]") {
     REQUIRE(total_weight == sketch.get_n());
   }
 
+  SECTION("estimation mode, exact min and max at ranks 0 and 1") {
+    const int n = 1000;
+    int num_min_dropped = 0;
+    int num_max_dropped = 0;
+    for (int t = 0; t < 1000; ++t) {
+      kll_float_sketch sketch(8, std::less<float>(), 0);
+      // min and max go in first, so compaction often drops them
+      sketch.update(0.0f);
+      sketch.update(static_cast<float>(n - 1));
+      for (int i = 1; i < n - 1; ++i) sketch.update(static_cast<float>(i));
+      bool min_retained = false;
+      bool max_retained = false;
+      for (auto pair: sketch) {
+        if (pair.first == 0) min_retained = true;
+        if (pair.first == n - 1) max_retained = true;
+      }
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == n - 1);
+      if (!min_retained) {
+        ++num_min_dropped;
+        REQUIRE(sketch.get_rank(0.0f) == 1.0 / n);
+      }
+      if (!max_retained) {
+        ++num_max_dropped;
+        REQUIRE(sketch.get_rank(static_cast<float>(n - 1), false) == (n - 1.0) / n);
+      }
+    }
+    REQUIRE(num_min_dropped > 0);
+    REQUIRE(num_max_dropped > 0);
+  }
+
+  SECTION("estimation mode, exact min and max at ranks 0 and 1, strings") {
+    int num_dropped = 0;
+    for (int t = 0; t < 1000; ++t) {
+      kll_string_sketch sketch(8, std::less<std::string>(), 0);
+      sketch.update(std::string("a"));
+      sketch.update(std::string("z"));
+      for (int i = 0; i < 1000; ++i) sketch.update("m" + std::to_string(i));
+      bool min_retained = false;
+      bool max_retained = false;
+      for (auto pair: sketch) {
+        if (pair.first == "a") min_retained = true;
+        if (pair.first == "z") max_retained = true;
+      }
+      if (!min_retained || !max_retained) ++num_dropped;
+      REQUIRE(sketch.get_quantile(0) == "a");
+      REQUIRE(sketch.get_quantile(0, false) == "a");
+      REQUIRE(sketch.get_quantile(1) == "z");
+      REQUIRE(sketch.get_quantile(1, false) == "z");
+    }
+    REQUIRE(num_dropped > 0);
+  }
+
+  SECTION("estimation mode, exact min and max at ranks 0 and 1 after deserialize and merge") {
+    const int n = 1000;
+    int num_dropped = 0;
+    for (int t = 0; t < 1000; ++t) {
+      kll_float_sketch sketch1(8, std::less<float>(), 0);
+      kll_float_sketch sketch2(8, std::less<float>(), 0);
+      // sketch1 gets the min first and sketch2 gets the max first, so compaction often drops them
+      sketch1.update(0.0f);
+      for (int i = 1; i < n; ++i) sketch1.update(static_cast<float>(i));
+      sketch2.update(static_cast<float>(2 * n - 1));
+      for (int i = n; i < 2 * n - 1; ++i) sketch2.update(static_cast<float>(i));
+
+      auto bytes = sketch1.serialize();
+      auto sketch = kll_float_sketch::deserialize(bytes.data(), bytes.size(), serde<float>(), std::less<float>(), 0);
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == n - 1);
+
+      sketch.merge(sketch2);
+      bool min_retained = false;
+      bool max_retained = false;
+      for (auto pair: sketch) {
+        if (pair.first == 0) min_retained = true;
+        if (pair.first == 2 * n - 1) max_retained = true;
+      }
+      if (!min_retained || !max_retained) ++num_dropped;
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == 2 * n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == 2 * n - 1);
+    }
+    REQUIRE(num_dropped > 0);
+  }
+
   SECTION("consistency between get_rank and get_PMF/CDF") {
     kll_float_sketch sketch(200, std::less<float>(), 0);
     const int n = 200;

quantiles/include/quantiles_sketch.hpp

@@ -288,6 +288,9 @@ class quantiles_sketch {
    * of a hypothetical sorted version of the input stream so far.
    * <p>
    * If the sketch is empty this throws std::runtime_error.
+   * <p>
+   * For non-arithmetic types, the returned reference refers to an item of this sketch, so it
+   * must not be used after this sketch is updated, merged, assigned to, moved or destroyed.
    *
    * @param rank the specified normalized rank in the hypothetical sorted stream.
    * @param inclusive if true the weight of the given item is included into the rank.
@@ -475,7 +478,9 @@ class quantiles_sketch {
   const_iterator end() const;
 
   /**
-   * Gets the sorted view of this sketch
+   * Gets the sorted view of this sketch.
+   * For non-arithmetic types, the view refers to the items of this sketch, so it must not be used
+   * after this sketch is updated, merged, assigned to, moved or destroyed.
    * @return the sorted view of this sketch
    */
   quantiles_sorted_view<T, Comparator, Allocator> get_sorted_view() const;

quantiles/include/quantiles_sketch_impl.hpp

@@ -746,6 +746,7 @@ quantiles_sorted_view<T, C, A> quantiles_sketch<T, C, A>::get_sorted_view() cons
   }
 
   view.convert_to_cummulative();
+  if (!is_empty()) view.include_min_max(*min_item_, *max_item_);
   return view;
 }
 

quantiles/test/quantiles_sketch_test.cpp

@@ -218,6 +218,75 @@ TEST_CASE("quantiles sketch", "[quantiles_sketch]") {
     REQUIRE(total_weight == sketch.get_n());
   }
 
+  SECTION("estimation mode, exact min and max at ranks 0 and 1") {
+    const int n = 1000;
+    int num_min_dropped = 0;
+    int num_max_dropped = 0;
+    for (int t = 0; t < 1000; ++t) {
+      quantiles_float_sketch sketch(8, std::less<float>(), 0);
+      // min and max go in first, so compaction often drops them
+      sketch.update(0.0f);
+      sketch.update(static_cast<float>(n - 1));
+      for (int i = 1; i < n - 1; ++i) sketch.update(static_cast<float>(i));
+      bool min_retained = false;
+      bool max_retained = false;
+      for (auto pair: sketch) {
+        if (pair.first == 0) min_retained = true;
+        if (pair.first == n - 1) max_retained = true;
+      }
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == n - 1);
+      if (!min_retained) {
+        ++num_min_dropped;
+        REQUIRE(sketch.get_rank(0.0f) == 1.0 / n);
+      }
+      if (!max_retained) {
+        ++num_max_dropped;
+        REQUIRE(sketch.get_rank(static_cast<float>(n - 1), false) == (n - 1.0) / n);
+      }
+    }
+    REQUIRE(num_min_dropped > 0);
+    REQUIRE(num_max_dropped > 0);
+  }
+
+  SECTION("estimation mode, exact min and max at ranks 0 and 1 after deserialize and merge") {
+    const int n = 1000;
+    int num_dropped = 0;
+    for (int t = 0; t < 1000; ++t) {
+      quantiles_float_sketch sketch1(8, std::less<float>(), 0);
+      quantiles_float_sketch sketch2(8, std::less<float>(), 0);
+      // sketch1 gets the min first and sketch2 gets the max first, so compaction often drops them
+      sketch1.update(0.0f);
+      for (int i = 1; i < n; ++i) sketch1.update(static_cast<float>(i));
+      sketch2.update(static_cast<float>(2 * n - 1));
+      for (int i = n; i < 2 * n - 1; ++i) sketch2.update(static_cast<float>(i));
+
+      auto bytes = sketch1.serialize();
+      auto sketch = quantiles_float_sketch::deserialize(bytes.data(), bytes.size(), serde<float>(),
+          std::less<float>(), 0);
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == n - 1);
+
+      sketch.merge(sketch2);
+      bool min_retained = false;
+      bool max_retained = false;
+      for (auto pair: sketch) {
+        if (pair.first == 0) min_retained = true;
+        if (pair.first == 2 * n - 1) max_retained = true;
+      }
+      if (!min_retained || !max_retained) ++num_dropped;
+      REQUIRE(sketch.get_quantile(0) == 0);
+      REQUIRE(sketch.get_quantile(0, false) == 0);
+      REQUIRE(sketch.get_quantile(1) == 2 * n - 1);
+      REQUIRE(sketch.get_quantile(1, false) == 2 * n - 1);
+    }
+    REQUIRE(num_dropped > 0);
+  }
+
   SECTION("consistency between get_rank and get_PMF/CDF") {
     quantiles_float_sketch sketch(64, std::less<float>(), 0);
     const int n = 1000;

req/include/req_compactor.hpp

@@ -51,8 +51,6 @@ class req_compactor {
   T* begin();
   T* end();
 
-  uint64_t compute_weight(const T& item, bool inclusive) const;
-
   template<typename FwdT>
   void append(FwdT&& item);
 

req/include/req_compactor_impl.hpp

@@ -184,15 +184,6 @@ uint8_t req_compactor<T, C, A>::get_lg_weight() const {
   return lg_weight_;
 }
 
-template<typename T, typename C, typename A>
-uint64_t req_compactor<T, C, A>::compute_weight(const T& item, bool inclusive) const {
-  if (!sorted_) const_cast<req_compactor*>(this)->sort(); // allow sorting as a side effect
-  auto it = inclusive ?
-      std::upper_bound(begin(), end(), item, comparator_) :
-      std::lower_bound(begin(), end(), item, comparator_);
-  return std::distance(begin(), it) << lg_weight_;
-}
-
 template<typename T, typename C, typename A>
 template<typename FwdT>
 void req_compactor<T, C, A>::append(FwdT&& item) {

req/include/req_sketch.hpp

@@ -291,6 +291,8 @@ class req_sketch {
    * Returns an approximate quantile of the given normalized rank.
    * The normalized rank must be in the range [0.0, 1.0] (both inclusive).
    * <p>If the sketch is empty this throws std::runtime_error.
+   * <p>For non-arithmetic types, the returned reference refers to an item of this sketch, so it
+   * must not be used after this sketch is updated, merged, assigned to, moved or destroyed.
    *
    * @param rank of an item in the hypothetical sorted stream.
    * @param inclusive if true, the given rank is considered inclusive (includes weight of an item)
@@ -419,7 +421,9 @@ class req_sketch {
   const_iterator end() const;
 
   /**
-   * Gets the sorted view of this sketch
+   * Gets the sorted view of this sketch.
+   * For non-arithmetic types, the view refers to the items of this sketch, so it must not be used
+   * after this sketch is updated, merged, assigned to, moved or destroyed.
    * @return the sorted view of this sketch
    */
   quantiles_sorted_view<T, Comparator, Allocator> get_sorted_view() const;