[stojkomilos]

What changed

kll_sketch, quantiles_sketch and req_sketch keep the exact min and max of the stream. But get_quantile(0) and get_quantile(1) read only the retained items, and compaction can drop the min and max from them. Then ranks 0 and 1 give a retained item, not the exact min and max:

kll_sketch<float> sketch(8);
sketch.update(0);
sketch.update(999);
for (int i = 1; i < 999; ++i) sketch.update(i);
sketch.get_min_item();   // 0
sketch.get_quantile(0);  // 23 (changes from run to run)
sketch.get_max_item();   // 999
sketch.get_quantile(1);  // 998

Since 6.0.0, datasketches-java returns the exact min and max here (apache/datasketches-java#547, IncludeMinMax). Its sorted views add the min and max back when the retained items do not hold them. So for the same data, the two libraries give different answers at ranks 0 and 1.

This PR ports that step to C++:

• get_sorted_view() of kll_sketch, quantiles_sketch and req_sketch adds a missing min and max, with the same arithmetic as Java. A missing min goes first, and the entry after it gives 1 of its weight to the min. A missing max goes last, and the entry before it gives 1 of its weight to the max. The total weight does not change. So get_quantile(), get_rank(), get_PMF(), get_CDF() and the sorted view iterator all see the min and max, as in Java.
• The levels are sorted, so the sketch finds out if the min is retained by comparing it with the first item of each level. If not, it adds the min with the new quantiles_sorted_view::add_min() before the other items, so no entry has to move. The new convert_to_cummulative(max_item) takes the weights for the min and max and appends a missing max. The view reserves room for both.
• quantiles_sorted_view::add() returns early for an empty range. Before, it merged the whole view into a new vector for each empty level, and a KLL sketch usually has one or two once n is much larger than k. It also skips the merge when the new items start at or after the last entry, and fills the new entries by index instead of push_back().
• Cost: get_sorted_view() of a KLL float sketch with n = 1M and k = 8 to 10000 is 14-56% faster than master with GCC 10, 6-35% faster with GCC 9, and 2-36% faster with clang 22. Only very small views are slower: 6-7 ns for 1 item, and 5 ns for 20 items with clang.
• REQ get_rank() now reads the cached sorted view, as in KLL, quantiles and Java ReqSketch. Before, it summed the weights in the compactors, so it did not see the min and max that only the view has. The view is built from the REQ compactors, so the REQ algorithm does not change: on master, the old get_rank() and the view agree in all 240,048 checks (HRA and LRA, k = 4, 12 and 50). This removes the only caller of req_compactor::compute_weight(), so it is removed.
• Cost of the REQ change: queries with no update in between are about 4x faster (k = 12: 204 ns to 54 ns per get_rank()). A get_rank() after each update() rebuilds the view and is about 20x slower (k = 12: 1.8 us to 36 us). That is the cost that get_quantile() already has after an update.

Behavior change: only ranks within 1/n of 0 or 1 give a different quantile, and a rank changes by at most 1/n. For example, the inclusive rank of a dropped min is now 1/n, not 0. These are the values that Java returns for the same data.

For non-arithmetic types, the view and the reference that get_quantile() returns now can point to the min and max in the sketch object, not only to its item buffer. So a move of the sketch now also makes them invalid. The docs of get_sorted_view(), get_quantile() and quantiles_sorted_view now say that, for non-arithmetic types, they must not be used after the sketch is updated, merged, assigned to, moved or destroyed.

How tested

Added

quantiles_sorted_view_test (7 tests): each checks the entries and their cumulative weights.

• Min and max (5 separate tests): both missing, both retained, one retained item (with the data of the Java IncludeMinMaxTest), only the min missing, and only the max missing.
• add() (2 separate tests): empty ranges first and between two ranges, and items that start at the last entry, where the earlier equal item stays first.

kll_sketch_test, quantiles_sketch_test, req_sketch_test (8 tests): each builds 1000 sketches, adds the min and max first so compaction drops them, and requires at least one sketch that dropped one. The REQ tests run in HRA and LRA.

• Built by update (KLL float and string, quantiles float, REQ float and string = 5 tests): ranks 0 and 1 give the exact min and max. The float tests also check the Java ranks of a dropped min and max.
• After deserialize and merge (KLL, quantiles, REQ = 3 tests): the same checks on a deserialized sketch and on its merge.

Test strength: without the fix, the 5 KLL and quantiles tests fail in 1000 of 1000 runs, and the 3 REQ tests fail. With add_min() and the max step of convert_to_cummulative(max_item) changed to do nothing, all 8 sketch tests and the 4 sorted view tests that need the min or max fail. "Both retained" and the 2 add() tests pass, as expected.

This pull request and its description were written by Isaac.

[leerho]

Thanks for this. Matching Java at ranks 0 and 1 is worthwhile, and the arithmetic in include_min_max() matches IncludeMinMax. CI passes, and the common, KLL and classic quantiles tests also pass for me locally under ASan and UBSan.

Timing: this changes the results users get, so it belongs in a minor release, not in the bug-fix release we're finishing now. We're holding other PRs until that release is out.

There are two things to settle before merging.

1. Sorted view lifetime. As you noted, for non-arithmetic types the view now points at the min and max inside the sketch object, so a view held across a move of the sketch no longer works. That fits the existing rule, as in Java, that a sorted view shouldn't be used after the sketch changes, but in C++ that rule isn't written down, and the failure is silent. Could you add it to the get_sorted_view() documentation in both sketches, saying that for non-arithmetic types the view refers to the sketch's items and must not be used after the sketch is updated, merged, moved or destroyed?

2. REQ. With this PR, KLL and classic quantiles match Java, but REQ doesn't. I'd like to see the REQ follow-up you offered planned alongside this, so all three sketches change in the same release. Moving REQ get_rank() to the sorted view is a reasonable way to do it.

[stojkomilos]

Removed as it's now unused.