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 convert_to_cummulative(const T& min_item, const T& max_item);
+
   class const_iterator;
 
   /**

common/include/quantiles_sorted_view_impl.hpp

@@ -32,7 +32,7 @@ comparator_(comparator),
 total_weight_(0),
 entries_(allocator)
 {
-  entries_.reserve(num);
+  entries_.reserve(num + 2); // room for the min and max
 }
 
 template<typename T, typename C, typename A>
@@ -60,6 +60,30 @@ 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).
+// The min is added in the cumulative pass, which shifts the entries without a second pass.
+template<typename T, typename C, typename A>
+void quantiles_sorted_view<T, C, A>::convert_to_cummulative(const T& min_item, const T& max_item) {
+  if (comparator_(deref_helper(entries_.back().first), max_item)) {
+    --entries_.back().second;
+    entries_.push_back(Entry(ref_helper(max_item), 1));
+  }
+  if (!comparator_(min_item, deref_helper(entries_.front().first))) {
+    convert_to_cummulative();
+    return;
+  }
+  --entries_.front().second;
+  entries_.push_back(entries_.back()); // the slot that the last entry shifts into
+  Entry carry(ref_helper(min_item), 1);
+  for (auto& entry: entries_) {
+    const Entry next = entry;
+    total_weight_ += carry.second;
+    entry = Entry(carry.first, total_weight_);
+    carry = next;
+  }
+}
+
 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,107 @@ 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(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(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(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);
+}
+
+TEST_CASE("include only max", "sorted view") {
+  auto view = quantiles_sorted_view<float, std::less<float>, std::allocator<float>>(3, std::less<float>(), std::allocator<float>());
+  std::vector<float> l1 {2, 4};
+  std::vector<float> l0 {6};
+  view.add(l1.begin(), l1.end(), 2);
+  view.add(l0.begin(), l0.end(), 1);
+  view.convert_to_cummulative(2, 8);
+  REQUIRE(view.size() == 4);
+
+  // 6 gives its only weight to the max
+  const std::vector<std::pair<float, uint64_t>> expected {{2, 2}, {4, 4}, {6, 4}, {8, 5}};
+  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 only min", "sorted view") {
+  auto view = quantiles_sorted_view<float, std::less<float>, std::allocator<float>>(3, std::less<float>(), std::allocator<float>());
+  std::vector<float> l1 {4, 6};
+  std::vector<float> l0 {2};
+  view.add(l1.begin(), l1.end(), 2);
+  view.add(l0.begin(), l0.end(), 1);
+  view.convert_to_cummulative(1, 6);
+  REQUIRE(view.size() == 4);
+
+  // 2 gives its only weight to the min
+  const std::vector<std::pair<float, uint64_t>> expected {{1, 1}, {2, 1}, {4, 3}, {6, 5}};
+  auto it = view.begin();
+  for (const auto& entry: expected) {
+    REQUIRE(it->first == entry.first);
+    REQUIRE(it->second == entry.second);
+    ++it;
+  }
+  REQUIRE(it == view.end());
+}
+
 } /* 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

@@ -774,7 +774,7 @@ quantiles_sorted_view<T, C, A> kll_sketch<T, C, A>::get_sorted_view() const {
     const auto to = items_ + levels_[level + 1]; // exclusive
     view.add(from, to, 1ULL << level);
   }
-  view.convert_to_cummulative();
+  if (!is_empty()) view.convert_to_cummulative(*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

@@ -745,7 +745,7 @@ quantiles_sorted_view<T, C, A> quantiles_sketch<T, C, A>::get_sorted_view() cons
     view.add(level.begin(), level.end(), weight);
   }
 
-  view.convert_to_cummulative();
+  if (!is_empty()) view.convert_to_cummulative(*min_item_, *max_item_);
   return view;
 }