jaideeppyne opened a new pull request, #254: URL: https://github.com/apache/datasketches-rust/pull/254
An empty `ThetaSketch` or `TupleSketch` built with `sampling_probability(p)` for `p < 1.0` reports the sampling theta instead of `MAX_THETA`: ``` p=0.1: theta64=922337217429372928 theta=0.1 is_estimation_mode=true ``` C++ and Java mask theta while the sketch is empty, so both report `theta64=MAX_THETA`, `theta=1.0`, `is_estimation_mode=false` for the same sketch. Java does it in `Sketch.isEstimationMode()` (`getThetaLong() < Long.MAX_VALUE && !isEmpty()`) and `CompactOperations.correctThetaOnCompact()`; C++ masks in `get_theta64()`. This repo already applies the same correction in `SketchHashTable::to_compact_parts`, which is why `compact()` gives the right answer and the plain accessor does not. The stale theta is not just cosmetic. `ThetaANotB`/`TupleANotB` copy A verbatim when A is empty, so the result is a sketch with `is_empty() == true` and `theta64() < MAX_THETA`. That serializes to a three-preamble-long image carrying an explicit theta, and our own deserializer drops the theta because the empty flag is set, so `serialize -> deserialize -> serialize` is not the identity: ``` a_not_b(empty p=0.5, empty p=0.5) -> 03030300001ecc9300000000000000000000000000000040 round trip -> 01030300001ecc93 ``` C++ emits the one-preamble-long form here and round trips cleanly. Fix is to mask theta to `MAX_THETA` while the sketch is empty, in `ThetaSketch::theta64`, `TupleSketch::theta64`, and `TupleSketchView::theta64` (the tuple view borrows the hash table directly, so it cannot go through the sketch accessor). `theta()` and `is_estimation_mode()` now derive from `theta64()`. I changed two existing assertions, and the old ones were wrong: `theta_test::test_bounds_empty_estimation_mode` and `tuple_test::empty_sampled_sketch_has_zero_bounds` both asserted `is_estimation_mode()` is true for an empty sampled sketch. Java's `isEstimationMode()` explicitly ANDs `!isEmpty()`, and the Python binding over the C++ core returns `False`, so those assertions encoded the bug. Their bound assertions are unchanged and still pass. How I found it: differential testing against the C++ core via `pip install datasketches`, plus oracle-free invariants. I swept 1296 Theta and 36 Tuple configurations over sampling probability x cardinality x ordering across `compact`, union, intersection and both a-not-b directions, asserting `is_empty() => theta64() == MAX_THETA` and serialize/deserialize identity. Before: violations in `a_not_b` and in the empty-sketch accessors for every `p < 1.0`. After: 0 violations in both families, and the accessor values match the C++ core exactly. The same sweep found nothing in union or intersection, which already handle empty inputs correctly, so this covers every reachable path I could construct rather than only the one I first hit. Fail-before / pass-after with only the source reverted and the new tests kept: 4 failures in `theta_test` and 4 in `tuple_test`. With the fix: full workspace green, including the 102 cross-language TCK serialization snapshot tests. Also checked and found clean while I was in here, so noting it to save someone else the trip: HLL query results match the C++ core on 7164 sketch and union images, and HLL union non-commutativity and non-idempotence reproduce identically in C++ (expected HIP-to-composite estimator switching, not a bug). AI assistance: written with Claude Code, reviewed and tested by me. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
