feat(vector3d, count-hll): fill in Vector3D + per-bucket-HLL CountHll sketch#62
Open
GordonYuanyc wants to merge 7 commits into
Open
feat(vector3d, count-hll): fill in Vector3D + per-bucket-HLL CountHll sketch#62GordonYuanyc wants to merge 7 commits into
GordonYuanyc wants to merge 7 commits into
Conversation
… sketch Flesh out the Vector3D<T> stub to full Vector2D parity: a rows x cols grid where each (row, col) cell is a contiguous `depth`-length bucket. Adds init/from_fn/fill, element + bucket accessors, fast_insert, the fast_query_min/median/max[_with_key] + aggregate family over bucket slices, Nitro hooks, custom serde (recomputes col mask), and Index/IndexMut. Add CountHll<H>: a Count Sketch grid whose cells are per-bucket HyperLogLog sketches (Vector3D<u8>, depth = 2^precision). Each item is routed to one column per row and recorded in that bucket's HLL. Supports estimate() (per-key distinct count, median across rows) and estimate_total_cardinality() (per-row column partition sum), plus register-max merge and msgpack serde. The HLL register/rank math and classic estimator mirror sketches::hll. Purely additive: Vector3D had no real callers, and only a new module + re-export were added. No existing API changed. fmt clean; new code is clippy-clean; all 489 lib + integration tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
cargo doc --no-deps -D warnings rejected [`Vector3D<u8>`] because rustdoc cannot resolve a path containing generics. Give the link an explicit target (crate::Vector3D) so the display text keeps the generic. Doc-comment only; no API change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Hash reuse: replace rows+1 hash64 calls per insert with 2 calls total (hash128 for packed column bits across all rows, hash64 with distinct seed for HLL register/rank); cols arg unchanged, no API change. - Bit-mask column selection: pre-compute col_mask at construction; when cols is a power-of-two, col_from_packed uses & instead of %. - Branchless register update: bucket[index].max(rank) compiles to cmov, eliminating the unpredictable branch on dense streams. - Single-pass estimate_bucket: fuse harmonic sum + zero count into one loop traversal, halving cache pressure; used rows*cols times in estimate_total_cardinality. - Stack-allocated median: use compute_median_inline_f64 over fixed-size [f64; 16] stack buffers instead of Vec+sort in estimate and estimate_total_cardinality; no heap allocation on the hot path. - Remove field duplication: rows/cols now read from Vector3D directly, eliminating deserialization skew risk. - Branchless register max in merge: (*reg).max(other_reg) replaces if. - Add two tests: bitmask/modulo path selection, insert_many parity. No pre-PR existing API changed. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
- Guard hash capacity: assert rows × col_mask_bits ≤ 128 in with_dimensions so configs that would cause shift overflow or silent bit-truncation are rejected at construction time instead of producing wrong column routing. - Fix silent row truncation in estimates: replace the [f64; 16] fixed buffers and rows.min(16) cap in estimate() and estimate_total_cardinality() with Vec<f64> so all rows always contribute; previously inserts updated rows 17+ but queries silently ignored them. - Fix serde derived-field trust: remove Deserialize from #[derive], add #[serde(skip)] to p_mask/col_mask_bits/col_mask, and implement a custom Deserialize (CountHllSeed pattern, matching Vector3D) that recomputes the three derived fields from the two authoritative fields (buckets, precision), with validation; stale or tampered wire bytes can no longer produce internally inconsistent hashing. Add three regression tests: too_many_rows_for_col_bits_panics, rows_beyond_16_contribute_to_estimates, deserialize_recomputes_derived_fields. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
insert(key, distinct_value) / estimate(key) replaces the single-input API where estimate(value) returned bucket cardinality — a number that depended only on stream size and col count, not on the query value. The new API answers "how many distinct values were seen for this key?", which is the meaningful use case for the Count-Sketch-backed HLL grid. estimate_total_cardinality() is removed: after the redesign a distinct value can appear in multiple buckets (once per key it is paired with), so the per-row column-sum no longer partitions cleanly. Also adds two missing deserialization invariant checks (Finding 1 from PR review): depth != 2^precision and rows*col_mask_bits > 128 are now rejected at deserialize time with the same guarantees as the constructor. Module docs gain a Related sketches section pointing to hll and Hydra. Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Vector2Dparity: arows × colsgrid where each(row, col)cell is adepth-length bucket. Adds init/from_fn/fill, element + bucket accessors, fast_insert, the fast_query_min/median/max[_with_key] + aggregate family over bucket slices, Nitro hooks, custom serde (recomputes col mask), Index/IndexMut.Vector3D<u8>, depth =2^precision). Answers grouped distinct-count queries:insert(key, distinct_value)records that a value was seen for a key;estimate(key)returns the estimated number of distinct values seen for that key (median across rows suppresses hash-collision noise). Plus register-maxmergeand msgpack serde.API impact
Purely additive —
Vector3Dhad no real callers; only a new module + re-export were added. No existing API changed.Fixes (PR review)
1 — API redesign:
insert(key, distinct_value)/estimate(key)The original single-input
insert(value)/estimate(value)API was meaningless:estimate(value)returned the cardinality of whichever hash bucketvaluelanded in, a number determined only by stream size and column count — the query value conveyed no information. The Count Sketch median across rows didn't suppress noise from other keys; it just averaged independent estimates of the same thing.The redesigned API routes by
key(hash128 → column selection) and recordsdistinct_valueinto the HLL registers (hash64 → register/rank).estimate(key)now answers "how many distinct values were seen for this key?" — the intended grouped distinct-count use case.estimate_total_cardinality()is removed: after the redesign a single distinct value can appear in multiple buckets (once per key it is paired with), so the per-row column-sum no longer partitions the stream.Related sketches are now documented in the module header:
sketches::hllfor total-stream distinct counting, andsketch_framework::hydra(HydrawithHydraCounter::HLL) for the same per-key distinct-count query with heap-allocated HLL objects per cell vs.CountHll's contiguousVector3D<u8>layout.2 — Hash capacity guard
with_dimensionsassertsrows × col_mask_bits ≤ 128at construction. Violated configurations previously caused shift overflow (debug) or silent bit-truncation (release); they now panic with a clear message.3 — All rows contribute to estimates
estimatepreviously used a[f64; 16]fixed buffer withrows.min(16), so inserts updated rows 17+ but queries silently ignored them. Now builds aVec<f64>over all rows.compute_median_inline_f64already has a sort-based fallback for lengths > 5.4 — Serde recomputes derived fields + validates invariants
Custom
DeserializeviaCountHllSeed(matching theVector3Dpattern) reads onlybucketsandprecisionfrom the wire and recomputesp_mask,col_mask_bits,col_mask. Now also validates on deserialization:buckets.depth() == 2^precision— a mismatched depth would cause out-of-bounds register access ininsertrows × col_mask_bits ≤ 128— same constraint as the constructorChecks
cargo fmt --all -- --checkcleanuninlined_format_argslint debt in unrelated files, unaffected by this PR)cargo test --all-features --locked: 479 lib + integration tests pass (14 CountHll-specific)🤖 Generated with Claude Code