fix(theta,tuple): canonicalize empty sampled sketch state - #254
Conversation
An empty sketch built with a sampling probability below 1.0 reported the sampling theta rather than MAX_THETA, so theta() returned p instead of 1.0 and is_estimation_mode() returned true. Java and C++ mask theta to MAX_THETA while the sketch is empty. The stale theta also reached ThetaANotB and TupleANotB results computed from an empty input, which serialized to a three-preamble-long image whose theta deserialization discarded, so those results did not survive a serialization round trip.
5dc86fb to
0b5d3c3
Compare
There was a problem hiding this comment.
Pull request overview
This PR aligns Rust Theta/Tuple sketch empty-state semantics with the C++/Java implementations by masking the externally reported theta while a sketch is empty (even when configured with sampling_probability(p), p < 1.0). It also refactors compact sketch production around a canonical compact-state representation so empty results serialize consistently and survive round trips without changing logical state.
Changes:
- Mask
theta64()toMAX_THETA(and derivetheta()/is_estimation_mode()from it) while sketches are logically empty. - Introduce
CompactSketchState+ThetaFamilySketchMetadatato standardize empty vs non-empty metadata across set operations and compact outputs. - Update integration tests and changelog to reflect the corrected empty-state behavior and canonical empty serialization.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests-integration/tests/tuple_test/union.rs | Adds assertions that empty union results report theta64 == MAX_THETA and is_estimation_mode == false, even with sampling configured. |
| tests-integration/tests/tuple_test/sketch.rs | Updates empty-sampled sketch expectations and adds round-trip / state-transition coverage around compaction + reset. |
| tests-integration/tests/tuple_test/a_not_b.rs | Extends empty-input A-not-B assertions to validate masked theta and exact-mode semantics. |
| tests-integration/tests/theta_test/union.rs | Ensures empty union outputs are ordered, exact-mode, and report MAX_THETA even under sampling. |
| tests-integration/tests/theta_test/sketch.rs | Fixes incorrect prior expectation for empty sampled sketches and adds serialization/state-transition checks. |
| tests-integration/tests/theta_test/intersection.rs | Adjusts intersection empty-result ordering/emptiness assertions to match canonical empty behavior. |
| tests-integration/tests/theta_test/a_not_b.rs | Validates a_not_b on empty sampled A yields canonical empty (MAX_THETA, not estimation mode). |
| tests-integration/src/lib.rs | Adds a shared MAX_THETA constant for integration tests. |
| datasketches/src/thetafamily/tuple/union.rs | Routes union output through canonical compact-state conversion. |
| datasketches/src/thetafamily/tuple/sketch.rs | Introduces logical emptiness tracking separate from retention theta; masks theta64() when empty; uses canonical compact state. |
| datasketches/src/thetafamily/tuple/intersection.rs | Uses canonical compact-state conversion for intersection results. |
| datasketches/src/thetafamily/tuple/a_not_b.rs | Uses canonical compact-state result for A-not-B. |
| datasketches/src/thetafamily/theta/union.rs | Converts union output via canonical compact state and retained-entry mapping. |
| datasketches/src/thetafamily/theta/sketch.rs | Adds logical emptiness tracking; masks theta64() when empty; uses canonical compact state for compaction/iteration/serialization. |
| datasketches/src/thetafamily/theta/intersection.rs | Converts intersection output via canonical compact state and retained-entry mapping. |
| datasketches/src/thetafamily/theta/a_not_b.rs | Converts A-not-B output via canonical compact state and retained-entry mapping. |
| datasketches/src/thetafamily/common/union.rs | Refactors union state to track result theta only once a non-empty input arrives; emits canonical empty state otherwise. |
| datasketches/src/thetafamily/common/sketch_state.rs | Adds shared ThetaFamilySketchMetadata and CompactSketchState to canonicalize empty/non-empty representation. |
| datasketches/src/thetafamily/common/mod.rs | Replaces scalar struct with shared metadata type and wires in the new sketch_state module. |
| datasketches/src/thetafamily/common/jaccard_similarity.rs | Updates Jaccard logic to consume the new metadata + compact-state APIs. |
| datasketches/src/thetafamily/common/intersection.rs | Refactors intersection operator to use explicit result-state tracking and canonical compact-state output. |
| datasketches/src/thetafamily/common/hash_table.rs | Renames theta handling to “retention theta” (operational screening threshold) and adjusts reset/compaction APIs accordingly. |
| datasketches/src/thetafamily/common/a_not_b.rs | Updates A-not-B computation to use metadata and canonical compact-state output; canonicalizes exact single-entry ordering. |
| CHANGELOG.md | Documents the empty-state theta/estimation-mode correction and canonical empty serialization behavior. |
Suppressed comments (1)
datasketches/src/thetafamily/common/hash_table.rs:467
starting_retention_theta()can return 0 for very small (but still > 0.0)sampling_probabilityvalues because the(MAX_THETA as f64 * p) as u64conversion truncates. Aretention_thetaof 0 makes every update get screened (hash >= 0), and it can also produce NaN estimates for non-empty sketches with zero retained entries (0.0 / 0.0).retention_thetashould always be in [1, MAX_THETA].
/// Computes the initial operational theta from a sampling probability.
pub fn starting_retention_theta(sampling_probability: f32) -> u64 {
if sampling_probability < 1.0 {
(MAX_THETA as f64 * sampling_probability as f64) as u64
} else {
MAX_THETA
}
}
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
@ZENOTME You may do a post-merge review and fix any regression or apply more ideas. |
Problem
A Theta-family update sketch configured with
sampling_probability < 1.0needs to represent two different facts:MAX_THETA;MAX_THETAand it must not report estimation mode.The previous representation stored emptiness, theta, retained entries, and ordering as independent fields. That allowed contradictory combinations such as
empty = truewiththeta < MAX_THETA.For an empty sampled sketch this produced:
It also affected set-operation results. In particular, A-not-B could copy an empty sampled input into an empty result that still carried the sampling theta. The result serialized with an explicit theta, but deserialization discarded it because the empty flag was set, so serialization was not stable across a round trip.
Java and C++ keep the sampling threshold internally but mask public theta while the sketch is empty. They report
theta64 = MAX_THETA,theta = 1.0, andis_estimation_mode = falsein this state.Design
This change separates operational hash-table state from the state represented by a sketch:
SketchHashTableowns onlyretention_theta, the threshold used to retain future updates. It no longer owns sketch emptiness.is_emptystate. An update call makes the sketch non-empty even when theta screens out the hash, so a non-empty sketch may legitimately retain zero entries.CompactSketchState<E>represents compact sketches as eitherEmpty { seed_hash }orNonEmpty { retained_entries, theta, seed_hash, ordered }. The empty variant cannot carry a stale theta, retained entries, or a contradictory ordering flag.ThetaFamilySketchMetadataapplies the same distinction at the set-operation boundary. Empty inputs expose only their seed hash; theta, ordering, and retained count exist only for non-empty inputs.Uninitialized/Empty/NonEmptystate machine. Union uses a local optional result theta, whereNonemeans that no non-empty input has been incorporated.The state types live in
thetafamily/common/sketch_state.rs, and callers import them from that owning module rather than through an internal facade re-export.Observable behavior
theta64 = MAX_THETA,theta = 1.0, andis_estimation_mode = falsefor every valid sampling probability.compact()preserves the distinction between a truly empty sketch and a non-empty sketch with zero retained entries.Tests
The regression coverage now focuses on observable state transitions rather than repeating the same assertions over probability matrices:
Validated locally with:
cargo x checkcargo x testAI assistance: Claude Code assisted the initial investigation and implementation. Codex assisted the subsequent review, state-model refactor, naming and test consolidation. The resulting changes were reviewed and validated by the authors.