diff --git a/common/include/quantiles_sorted_view.hpp b/common/include/quantiles_sorted_view.hpp index c3ab6af0..9528c9b8 100755 --- a/common/include/quantiles_sorted_view.hpp +++ b/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; /** diff --git a/common/include/quantiles_sorted_view_impl.hpp b/common/include/quantiles_sorted_view_impl.hpp index 3663950f..c8520376 100755 --- a/common/include/quantiles_sorted_view_impl.hpp +++ b/common/include/quantiles_sorted_view_impl.hpp @@ -60,6 +60,20 @@ void quantiles_sorted_view::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 +void quantiles_sorted_view::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 double quantiles_sorted_view::get_rank(const T& item, bool inclusive) const { if (entries_.empty()) throw std::runtime_error("operation is undefined for an empty sketch"); diff --git a/common/test/quantiles_sorted_view_test.cpp b/common/test/quantiles_sorted_view_test.cpp index 609408dd..938c4110 100644 --- a/common/test/quantiles_sorted_view_test.cpp +++ b/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, std::allocator>(4, std::less(), std::allocator()); + std::vector 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> 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, std::allocator>(4, std::less(), std::allocator()); + std::vector 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> 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, std::allocator>(1, std::less(), std::allocator()); + std::vector 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> 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 */ diff --git a/kll/include/kll_sketch.hpp b/kll/include/kll_sketch.hpp index d672c419..a397f3af 100644 --- a/kll/include/kll_sketch.hpp +++ b/kll/include/kll_sketch.hpp @@ -308,6 +308,9 @@ class kll_sketch { * *

If the sketch is empty this throws std::runtime_error. * + *

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 get_sorted_view() const; diff --git a/kll/include/kll_sketch_impl.hpp b/kll/include/kll_sketch_impl.hpp index b12a39c8..42891261 100644 --- a/kll/include/kll_sketch_impl.hpp +++ b/kll/include/kll_sketch_impl.hpp @@ -775,6 +775,7 @@ quantiles_sorted_view kll_sketch::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; } diff --git a/kll/test/kll_sketch_test.cpp b/kll/test/kll_sketch_test.cpp index b8d0b031..f6cd63ac 100644 --- a/kll/test/kll_sketch_test.cpp +++ b/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(), 0); + // min and max go in first, so compaction often drops them + sketch.update(0.0f); + sketch.update(static_cast(n - 1)); + for (int i = 1; i < n - 1; ++i) sketch.update(static_cast(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(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(), 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(), 0); + kll_float_sketch sketch2(8, std::less(), 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(i)); + sketch2.update(static_cast(2 * n - 1)); + for (int i = n; i < 2 * n - 1; ++i) sketch2.update(static_cast(i)); + + auto bytes = sketch1.serialize(); + auto sketch = kll_float_sketch::deserialize(bytes.data(), bytes.size(), serde(), std::less(), 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(), 0); const int n = 200; diff --git a/quantiles/include/quantiles_sketch.hpp b/quantiles/include/quantiles_sketch.hpp index e995e3e3..2e851ea5 100644 --- a/quantiles/include/quantiles_sketch.hpp +++ b/quantiles/include/quantiles_sketch.hpp @@ -288,6 +288,9 @@ class quantiles_sketch { * of a hypothetical sorted version of the input stream so far. *

* If the sketch is empty this throws std::runtime_error. + *

+ * 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 get_sorted_view() const; diff --git a/quantiles/include/quantiles_sketch_impl.hpp b/quantiles/include/quantiles_sketch_impl.hpp index 2dacf21e..96205912 100644 --- a/quantiles/include/quantiles_sketch_impl.hpp +++ b/quantiles/include/quantiles_sketch_impl.hpp @@ -746,6 +746,7 @@ quantiles_sorted_view quantiles_sketch::get_sorted_view() cons } view.convert_to_cummulative(); + if (!is_empty()) view.include_min_max(*min_item_, *max_item_); return view; } diff --git a/quantiles/test/quantiles_sketch_test.cpp b/quantiles/test/quantiles_sketch_test.cpp index 8230f6a1..198b6621 100644 --- a/quantiles/test/quantiles_sketch_test.cpp +++ b/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(), 0); + // min and max go in first, so compaction often drops them + sketch.update(0.0f); + sketch.update(static_cast(n - 1)); + for (int i = 1; i < n - 1; ++i) sketch.update(static_cast(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(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(), 0); + quantiles_float_sketch sketch2(8, std::less(), 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(i)); + sketch2.update(static_cast(2 * n - 1)); + for (int i = n; i < 2 * n - 1; ++i) sketch2.update(static_cast(i)); + + auto bytes = sketch1.serialize(); + auto sketch = quantiles_float_sketch::deserialize(bytes.data(), bytes.size(), serde(), + std::less(), 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(), 0); const int n = 1000; diff --git a/req/include/req_compactor.hpp b/req/include/req_compactor.hpp index f013e49d..7344f26b 100755 --- a/req/include/req_compactor.hpp +++ b/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 void append(FwdT&& item); diff --git a/req/include/req_compactor_impl.hpp b/req/include/req_compactor_impl.hpp index 72aafc94..03ed8126 100755 --- a/req/include/req_compactor_impl.hpp +++ b/req/include/req_compactor_impl.hpp @@ -184,15 +184,6 @@ uint8_t req_compactor::get_lg_weight() const { return lg_weight_; } -template -uint64_t req_compactor::compute_weight(const T& item, bool inclusive) const { - if (!sorted_) const_cast(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 template void req_compactor::append(FwdT&& item) { diff --git a/req/include/req_sketch.hpp b/req/include/req_sketch.hpp index 2e2bc309..2df727d7 100755 --- a/req/include/req_sketch.hpp +++ b/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). *

If the sketch is empty this throws std::runtime_error. + *

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 get_sorted_view() const; diff --git a/req/include/req_sketch_impl.hpp b/req/include/req_sketch_impl.hpp index 8acea8bc..06f926ff 100755 --- a/req/include/req_sketch_impl.hpp +++ b/req/include/req_sketch_impl.hpp @@ -237,11 +237,8 @@ A req_sketch::get_allocator() const { template double req_sketch::get_rank(const T& item, bool inclusive) const { if (is_empty()) throw std::runtime_error("operation is undefined for an empty sketch"); - uint64_t weight = 0; - for (const auto& compactor: compactors_) { - weight += compactor.compute_weight(item, inclusive); - } - return static_cast(weight) / n_; + setup_sorted_view(); + return sorted_view_->get_rank(item, inclusive); } template @@ -281,6 +278,7 @@ quantiles_sorted_view req_sketch::get_sorted_view() const { } view.convert_to_cummulative(); + if (!is_empty()) view.include_min_max(*min_item_, *max_item_); return view; } diff --git a/req/test/req_sketch_test.cpp b/req/test/req_sketch_test.cpp index c8cf5cbe..c6226b3b 100755 --- a/req/test/req_sketch_test.cpp +++ b/req/test/req_sketch_test.cpp @@ -213,6 +213,100 @@ TEST_CASE("req sketch: estimation mode", "[req_sketch]") { REQUIRE(count == sketch.get_num_retained()); } +TEST_CASE("req sketch: estimation mode, exact min and max at ranks 0 and 1", "[req_sketch]") { + const int n = 1000; + for (const bool hra: {true, false}) { + int num_dropped = 0; + for (int t = 0; t < 1000; ++t) { + req_sketch sketch(4, hra); + // min and max go in first, so compaction often drops one of them + sketch.update(0.0f); + sketch.update(static_cast(n - 1)); + for (int i = 1; i < n - 1; ++i) sketch.update(static_cast(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_dropped; + REQUIRE(sketch.get_rank(0.0f) == 1.0 / n); + } + if (!max_retained) { + ++num_dropped; + REQUIRE(sketch.get_rank(static_cast(n - 1), false) == (n - 1.0) / n); + } + } + REQUIRE(num_dropped > 0); + } +} + +TEST_CASE("req sketch: estimation mode, exact min and max at ranks 0 and 1, strings", "[req_sketch]") { + for (const bool hra: {true, false}) { + int num_dropped = 0; + for (int t = 0; t < 1000; ++t) { + req_sketch sketch(4, hra); + 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); + } +} + +TEST_CASE("req sketch: exact min and max at ranks 0 and 1 after deserialize and merge", "[req_sketch]") { + const int n = 1000; + for (const bool hra: {true, false}) { + int num_dropped = 0; + for (int t = 0; t < 1000; ++t) { + req_sketch sketch1(4, hra); + req_sketch sketch2(4, hra); + // 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(i)); + sketch2.update(static_cast(2 * n - 1)); + for (int i = n; i < 2 * n - 1; ++i) sketch2.update(static_cast(i)); + + auto bytes = sketch1.serialize(); + auto sketch = req_sketch::deserialize(bytes.data(), bytes.size()); + 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); + } +} + TEST_CASE("req sketch: stream serialize-deserialize empty", "[req_sketch]") { req_sketch sketch(12);