tisonkun opened a new issue, #188: URL: https://github.com/apache/datasketches-rust/issues/188
## Problem A purge can remove every active counter while leaving positive `stream_weight` and `offset`. `FrequentItemsSketch::is_empty()` is defined from the number of active counters, so both serialization and merge currently treat this non-virgin state as an unused sketch. A reproducible case with maximum map size 32 has the following state after inserting 25 distinct count-one items: - `num_active_items() == 0` - `is_empty() == true` - `total_weight() == 25` - `maximum_error() == 1` Serialization emits the eight-byte virgin-empty representation. After a round trip, total weight and maximum error are both zero. Merging this sketch is also a no-op. For an untracked item, the upper bound changes from 1 to 0 and no longer covers its true count of 1. Reviewed revision: `4266ad9cad21e0660c2ae9506174e01665bd100d`. ## Expected behavior Serialization and merge must distinguish a virgin stream from a sketch that currently has no active counters but retains stream/error state. Public observations relevant to frequency bounds must survive a Rust round trip and merge. The public `is_empty()` behavior does not need to change as part of this fix; retaining its active-item meaning minimizes API and cross-language divergence. Internal persistence and merge decisions should use a virgin-state predicate instead. ## Relationship to other implementations The current Java implementation defines empty from active items, uses that result to emit the short empty image, and skips such sketches during merge: https://github.com/apache/datasketches-java/blob/4067ffefabbcd03944dc3617ff2948ab760f3b75/src/main/java/org/apache/datasketches/frequencies/FrequentItemsSketch.java#L455-L477 https://github.com/apache/datasketches-java/blob/4067ffefabbcd03944dc3617ff2948ab760f3b75/src/main/java/org/apache/datasketches/frequencies/FrequentItemsSketch.java#L496-L533 Current C++ and Go have the same state model and loss: https://github.com/apache/datasketches-cpp/blob/c22888581964fc490feee835766cc8c3adb722e0/fi/include/frequent_items_sketch_impl.hpp#L68-L93 https://github.com/apache/datasketches-cpp/blob/c22888581964fc490feee835766cc8c3adb722e0/fi/include/frequent_items_sketch_impl.hpp#L165-L209 https://github.com/apache/datasketches-go/blob/c558cc2d64f9a307c196a7adb0067eadd1b776f7/frequencies/items_sketch.go#L327-L408 https://github.com/apache/datasketches-go/blob/c558cc2d64f9a307c196a7adb0067eadd1b776f7/frequencies/items_sketch.go#L442-L481 This is therefore a shared reference-family defect rather than a Rust-only wire-format mismatch. The existing non-empty preamble can represent zero active items together with stream weight and offset, and the current Java, C++, and Go deserializers accept an active-item count of zero. They still consider the result empty and may collapse it again on their own reserialization or merge, so the PR must document that interoperability limitation rather than claiming the other implementations are fixed. ## Proposed fix Introduce an internal virgin-state check based on stream history. Use it for the serialization and merge fast paths while retaining the public active-item definition of `is_empty()`. A purged zero-active sketch should therefore use the standard non-empty preamble with `active_items == 0`, preserving `stream_weight` and `offset` without defining a new wire format. ## Acceptance criteria - Add a deterministic regression setup that purges to zero active counters while retaining non-zero stream weight and offset. - Verify serialization round trips preserve total weight, maximum error, and upper bounds. - Verify merging the state into another sketch preserves the same observations. - Retain the existing eight-byte representation for a genuinely virgin sketch. - Explain the shared Java/C++/Go behavior and the remaining cross-language reserialization/merge limitation in the PR. - Run the repository check, test, and lint workflows. -- 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]
