diff --git a/docs/api/api_univmon.md b/docs/api/api_univmon.md index 3e5f120f..b023506d 100644 --- a/docs/api/api_univmon.md +++ b/docs/api/api_univmon.md @@ -17,6 +17,16 @@ fn default() -> Self fn init_univmon(heap_size: usize, sketch_row: usize, sketch_col: usize, layer_size: usize) -> Self ``` +`init_univmon` panics on a shape whose hashes would not be independent: + +- `layer_size > MAX_LAYER_SIZE` (19). Layer `i` hashes at seed index `i`, and + the layers a key reaches are read from the hash at `BOTTOM_LAYER_FINDER` + (19). A 20th layer would hash at the finder's seed, and later ones would wrap + onto layer 0's. +- `sketch_row * (ceil_log2(sketch_col) + 1) > 128`. Every row of a layer reads + its column and its sign from one 128-bit hash; past this budget a deep row's + column reuses a shallow row's sign bit. + ## Insert/Update ```rust diff --git a/docs/tests.md b/docs/tests.md index ccb1571c..af47e619 100644 --- a/docs/tests.md +++ b/docs/tests.md @@ -327,6 +327,8 @@ Wire tests: [`src/sketches/countsketch_topk/l2hh_wire.rs`](../src/sketches/count | test_name | test_description | what_is_tested | | --- | --- | --- | +| `countl2hh_rejects_column_bits_that_reach_the_sign_bits` | A geometry whose column and sign fields overlap panics at construction. | Verifies `CountL2HH::with_dimensions_and_seed(11, 2048, 0)` - 132 bits - panics with a message naming the hash bits. | +| `countl2hh_accepts_the_widest_disjoint_geometry` | The widest disjoint geometry is accepted. | Verifies `CountL2HH::with_dimensions_and_seed(10, 2048, 0)` - 120 bits - constructs. | | `countl2hh_estimates_and_l2_are_consistent` | CountL2HH updates keep estimate and L2 consistent. | For `CountL2HH(3x32)`, applies `+5` then `-2` to one key, verifies estimates `5.0` then `3.0`, and asserts non-trivial L2 (`>= 3.0`). | | `countl2hh_merge_combines_frequency_vectors` | CountL2HH merge combines per-key frequencies. | Merges two `CountL2HH(3x32)` sketches with same key counts `4` and `9`, then verifies merged estimate `== 13.0`. | | `countl2hh_round_trip_serialization` | CountL2HH serialization round trip preserves estimate and L2. | Serializes/deserializes `CountL2HH::with_dimensions_and_seed(3,32,7)` after updates, verifying rows/cols and that both estimate and L2 remain unchanged (within `f64::EPSILON`). | @@ -840,6 +842,9 @@ Wire tests: [`src/sketch_framework/univmon/wire.rs`](../src/sketch_framework/uni | `univmon_layers_with_different_heap_loads_round_trip` | Each layer's heap contents and emitted order survive the round trip. | For an `init_univmon(8, 2, 16, 4)` pyramid fed 40 weighted keys, verifies the layers carry different heap loads, that every layer's length, capacity of `8`, and per-key counts match after a decode, and that the bytes re-serialize identically. | | `univmon_carries_update_mode_and_candidate_flags` | `update_mode` and `candidate_complete` are state, not derived. | For a terminal-only `init_univmon(2, 2, 8, 3)` pyramid fed 20 keys through `fast_insert`, verifies the payload carries `update_mode` `2`, that the decode preserves the candidate flags, cardinality, and entropy and re-serializes identically, and that forcing every flag true changes what the pyramid reports. | | `univmon_empty_has_one_encoding` | An empty pyramid has exactly one encoding. | Verifies a fresh `init_univmon(4, 2, 16, 3)` pyramid and one freed after an insert serialize to identical bytes carrying the pinned `EMPTY_KEY_TYPE`, and that the decode holds empty heaps and re-serializes identically. | +| `layer_seeds_are_distinct_and_never_the_layer_finder` | Every layer hashes at its own seed, never the layer finder's. | For `init_univmon(4, 2, 16, MAX_LAYER_SIZE)`, verifies every layer's seed index is inside `SEEDLIST`, differs from `BOTTOM_LAYER_FINDER`, and is distinct from every other layer's. | +| `init_univmon_rejects_a_layer_on_the_finder_seed` | A layer at the finder's seed index is refused. | Verifies `init_univmon(4, 2, 16, BOTTOM_LAYER_FINDER + 1)` panics naming `MAX_LAYER_SIZE`. | +| `init_univmon_rejects_column_bits_that_reach_the_sign_bits` | A layer geometry past the hash budget is refused. | Verifies `init_univmon(4, 11, 2048, 2)` panics naming the hash bits. | | `univmon_pins_its_hash_profile` | UnivMon hashes through the crate default, so it has one truthful profile. | Verifies a populated pyramid emits `DefaultXxHasher`'s profile id and seed list, and that the same payload re-framed under a custom profile is different bytes that fail to decode. | | `univmon_rejects_foreign_kind_ids` | The neighbouring universal-sketch kind_ids are refused. | Verifies `Count`, `CountL2HH`, `UnivMonPyramid`, and `UnivMonQ` envelopes each fail to decode as a `UnivMon`. | | `univmon_rejects_crafted_shapes` | Every shape rule fires before an allocation is sized from it. | Verifies six crafted metadata shapes fail - a `layer_size` of `u32::MAX` or `0`, a zero `sketch_col` or `heap_size`, a `MATRIX_MAX_ROWS x 4096` sketch, and a `heap_size` of `1` - and that a short `heap_counts`, a short `candidate_complete`, and an `update_mode` of `7` each fail too. | @@ -1036,7 +1041,8 @@ Runtime feature: the `runtime_tests` module is `#[cfg(all(test, feature = "octo- | `dd_promotion_bounds_the_quantile_rank_error_by_what_it_holds_back` | The held-back mass bounds the parent's quantile rank error. | Feeds 200,000 values `1.0 + (i * 7) % 999` through a `DdWorkerSketch` at alpha `0.01` and a threshold of `DD_PROMASK` into a parent `DDSketch`, and verifies `held_back()` plus the parent's `get_count()` equals `200_000`, that the held-back share is under `0.05`, and that the parent's `0.1`, `0.5`, `0.9` and `0.99` quantiles each fall between the exact value at rank `q - slack` scaled by `1 - alpha` and the exact value at rank `q + slack` scaled by `1 + alpha`. | | `dd_a_higher_threshold_holds_back_more_of_a_sparse_stream` | Held-back mass grows with the threshold. | Over 20,000 values `1.0 + (i * 7) % 999` at alpha `0.01`, verifies `held_back()` is strictly increasing across thresholds `2`, `8` and `31`. | | `a_univmon_worker_refuses_a_geometry_that_outruns_the_hash` | A geometry past the 128-bit hash budget panics at construction. | Verifies `L2hhWorkerSketch::new(13, 2048, 0)` - 13 rows at 11 column bits, so 143 bits - panics with a message naming the 128-bit hash budget. | -| `the_widest_accepted_univmon_geometry_still_addresses_distinct_rows` | The widest geometry inside the budget still gives every row its own index. | For `L2hhWorkerSketch::new(11, 2048, 0)` - 121 bits - inserts `U64(7)` at a threshold of `1` and verifies `11` deltas landing on `11` distinct row indices. | +| `a_univmon_worker_refuses_column_bits_that_reach_the_sign_bits` | Column fields reaching the sign bits panic at construction. | Verifies `L2hhWorkerSketch::new(11, 2048, 0)` - 11 rows of 11 column bits plus 1 sign bit, so 132 bits - panics with a message naming the 128-bit hash budget. | +| `the_widest_accepted_univmon_geometry_still_addresses_distinct_rows` | The widest geometry inside the budget still gives every row its own index. | For `L2hhWorkerSketch::new(10, 2048, 0)` - 120 bits counting signs - inserts `U64(7)` at a threshold of `1` and verifies `10` deltas landing on `10` distinct row indices. | | `every_worker_reads_one_threshold_the_same_way` | The compact worker and the full sketch clamp one threshold identically. | Inserts `U64(9)` 400 times at a threshold of `200` into both a `1x64` `CountWorkerSketch` and a `1x64` regular-path `Count>`, and verifies both promoted at all, that the two emitted value sequences are equal, and that every emitted value's magnitude is `MAX_PROMASK`. | | `an_hll_worker_refuses_a_threshold_no_register_can_reach` | A threshold no register gain can reach panics at construction. | Verifies `HllOctoWorker::with_threshold(max_hll_threshold(14))` panics with a message naming the precision. | | `any_hll_threshold_above_zero_costs_cardinality_accuracy` | Only a threshold of zero leaves the parent's cardinality intact. | Over 50,000 distinct keys into a default `HyperLogLog` child and parent, sweeps thresholds `0`, `1`, `2` and `4` and verifies the threshold-`0` parent estimates exactly what a single-threaded reference does, that each higher threshold sends strictly fewer messages and estimates no higher than the threshold below it, that the threshold-`4` estimate is under half of 50,000, and that `max_hll_threshold(14)` is `50`. | diff --git a/src/sketch_framework/octo.rs b/src/sketch_framework/octo.rs index 332d42bd..fa32303d 100644 --- a/src/sketch_framework/octo.rs +++ b/src/sketch_framework/octo.rs @@ -41,7 +41,7 @@ use crate::octo_delta::{ use crate::sketch_framework::univmon::{UnivMonDeltaFidelity, bottom_layer_for_hash}; use crate::sketches::coco::{Coco, CocoRng}; use crate::sketches::countminsketch_topk::CMSHeap; -use crate::sketches::countsketch_topk::{CSHeap, l2hh_cell_for_row}; +use crate::sketches::countsketch_topk::{CSHeap, check_l2hh_hash_bits, l2hh_cell_for_row}; use crate::sketches::elastic::{Elastic, LAMBDA}; use crate::{ BOTTOM_LAYER_FINDER, CM_PROMASK, COUNT_PROMASK, Classic, CmDelta, Count, CountDelta, CountMin, @@ -411,13 +411,13 @@ impl L2hhWorkerSketch { let rows = rows.max(1); let cols = cols.max(1); let mask_bits = cols_mask_bits(cols); - // Every row slices its column out of the same 128-bit hash, so the - // rows collectively have 128 bits to spend. Past that the shift is - // undefined: debug builds panic and release builds wrap, aliasing a - // deep row onto a shallow one's column bits. + // Every row slices its column and its sign out of the same 128-bit + // hash, so the rows collectively have 128 bits to spend. Past that a + // deep row's column reads a shallow row's sign bit, and further on + // the shift runs off the hash. assert!( - rows * mask_bits as usize <= 128, - "{rows} rows x {mask_bits} column bits exceeds the 128-bit hash budget; \ + check_l2hh_hash_bits(rows, cols).is_ok(), + "{rows} rows x ({mask_bits} column bits + 1 sign bit) exceeds the 128-bit hash budget; \ reduce rows or columns" ); Self { @@ -3088,15 +3088,23 @@ mod worker_tests { L2hhWorkerSketch::new(13, 2048, 0); } + /// 11 rows x (11 column bits + 1 sign bit) = 132: the column fields of + /// the deep rows would read the shallow rows' sign bits. + #[test] + #[should_panic(expected = "exceeds the 128-bit hash budget")] + fn a_univmon_worker_refuses_column_bits_that_reach_the_sign_bits() { + L2hhWorkerSketch::new(11, 2048, 0); + } + #[test] fn the_widest_accepted_univmon_geometry_still_addresses_distinct_rows() { - // 11 rows x 11 bits = 121, just inside the budget. - let mut worker = L2hhWorkerSketch::new(11, 2048, 0); + // 10 rows x (11 column bits + 1 sign bit) = 120, inside the budget. + let mut worker = L2hhWorkerSketch::new(10, 2048, 0); let mut cells: Vec<(u32, u32)> = Vec::new(); worker.insert_emit_delta(&DataInput::U64(7), 1, &mut |d| cells.push((d.row, d.col))); - assert_eq!(cells.len(), 11, "every row promotes at threshold 1"); + assert_eq!(cells.len(), 10, "every row promotes at threshold 1"); let rows: std::collections::HashSet = cells.iter().map(|(r, _)| *r).collect(); - assert_eq!(rows.len(), 11, "each row must land on its own index"); + assert_eq!(rows.len(), 10, "each row must land on its own index"); } #[test] diff --git a/src/sketch_framework/univmon.rs b/src/sketch_framework/univmon.rs index f13f666b..cdfa9886 100644 --- a/src/sketch_framework/univmon.rs +++ b/src/sketch_framework/univmon.rs @@ -25,7 +25,7 @@ use crate::common::heap::HHHeap; use crate::common::{ - BOTTOM_LAYER_FINDER, DataInput, HeapItem, hash_item64_seeded, hash64_seeded, + BOTTOM_LAYER_FINDER, DataInput, HeapItem, SEEDLIST, hash_item64_seeded, hash64_seeded, heap_item_to_sketch_input, }; use crate::common::{L2HH, Vector1D}; @@ -83,10 +83,19 @@ impl Default for UnivMon { } } -/// Layers one pyramid may hold. [`bottom_layer_for_hash`] and the query -/// recurrences shift a 64-bit key hash right by up to `layer_size - 1`, so a -/// deeper pyramid would shift past the hash's width. -pub const MAX_LAYER_SIZE: usize = 64; +/// Layers one pyramid may hold. +/// +/// Layer `i`'s Count Sketch hashes at seed index `i`, and the layer a key +/// reaches is read from the hash at seed index [`BOTTOM_LAYER_FINDER`]. The +/// error bounds need every layer's hash independent of the bits that select +/// its keys and of the other layers' hashes, so no layer may hash at the +/// finder's seed, and none past the seed list, where indices wrap onto layer +/// 0's. Layers `0..BOTTOM_LAYER_FINDER` are exactly the distinct seeds below +/// the finder. (The finder's bits are read by shifting a 64-bit hash right by +/// up to `layer_size - 1`, which this cap also keeps in range.) +pub const MAX_LAYER_SIZE: usize = BOTTOM_LAYER_FINDER; + +const _: () = assert!(BOTTOM_LAYER_FINDER < SEEDLIST.len() && MAX_LAYER_SIZE < 64); /// Deepest pyramid layer the given key hash reaches. #[inline(always)] @@ -557,6 +566,42 @@ mod tests { use rand::{Rng, SeedableRng, rngs::StdRng}; use std::collections::HashMap; + /// Every layer hashes at its own seed, inside the seed list (no wrap onto + /// layer 0) and never at the seed that picks the layers. + #[test] + fn layer_seeds_are_distinct_and_never_the_layer_finder() { + use crate::common::SEEDLIST; + let deepest = UnivMon::init_univmon(4, 2, 16, MAX_LAYER_SIZE); + let seeds: Vec = (0..MAX_LAYER_SIZE) + .map(|layer| { + let L2HH::COUNT(counter) = &deepest.l2_sketch_layers[layer]; + counter.seed_idx() + }) + .collect(); + for &seed in &seeds { + assert!(seed < SEEDLIST.len(), "seed index {seed} wraps"); + assert_ne!( + seed, BOTTOM_LAYER_FINDER, + "a layer hashes at the finder seed" + ); + } + let distinct: HashSet = seeds.iter().copied().collect(); + assert_eq!(distinct.len(), seeds.len(), "two layers share a seed"); + } + + /// Layer `BOTTOM_LAYER_FINDER` would hash at the finder's own seed. + #[test] + #[should_panic(expected = "MAX_LAYER_SIZE")] + fn init_univmon_rejects_a_layer_on_the_finder_seed() { + UnivMon::init_univmon(4, 2, 16, BOTTOM_LAYER_FINDER + 1); + } + + #[test] + #[should_panic(expected = "hash bits")] + fn init_univmon_rejects_column_bits_that_reach_the_sign_bits() { + UnivMon::init_univmon(4, 11, 2048, 2); + } + /// Persisted terminal states must be distinguishable before standard ingestion. #[test] fn standard_update_compatibility_survives_roundtrip_and_reset() { diff --git a/src/sketches/countsketch_topk.rs b/src/sketches/countsketch_topk.rs index 5c2e2501..c3c5d0ea 100644 --- a/src/sketches/countsketch_topk.rs +++ b/src/sketches/countsketch_topk.rs @@ -9,6 +9,7 @@ use serde::{Deserialize, Serialize}; use std::marker::PhantomData; +use crate::common::structures::matrix_storage::cols_mask_bits; use crate::octo_delta::CountDelta; use crate::sketches::countsketch::CountSketchCounter; use crate::{ @@ -939,6 +940,27 @@ mod tests { // file stays focused on plain Count Sketch. // ===================================================================== +/// Checks that a `rows` x `cols` `CountL2HH` fits its one 128-bit key hash. +/// +/// Row `r` reads its column from bits `[r * mask_bits, (r + 1) * mask_bits)` +/// and its sign from bit `127 - r`, where `mask_bits = cols_mask_bits(cols)`. +/// Each row therefore claims `mask_bits + 1` bits, and the fields are in range +/// and pairwise disjoint (so every column and sign is an independent hash +/// draw) exactly while `rows * (mask_bits + 1) <= 128`. Past that, deep rows' +/// columns reuse shallow rows' sign bits, and from `rows * mask_bits >= 128` +/// the shift runs off the hash. +pub(crate) fn check_l2hh_hash_bits(rows: usize, cols: usize) -> Result<(), String> { + let mask_bits = cols_mask_bits(cols.max(1)) as usize; + let bits_required = mask_bits.saturating_add(1).saturating_mul(rows); + if bits_required > 128 { + return Err(format!( + "CountL2HH rows={rows}, cols={cols} needs {bits_required} hash bits \ + ({mask_bits} column bits + 1 sign bit per row), past the 128 the key hash carries" + )); + } + Ok(()) +} + /// Column and sign `CountL2HH` uses for `row`, given its 128-bit key hash. /// /// Shared with the OctoSketch worker sketches so a worker and its parent can @@ -985,7 +1007,15 @@ impl CountL2HH { } /// Creates a sketch with the requested dimensions and a custom seed offset. + /// + /// # Panics + /// + /// When `rows * (cols_mask_bits(cols) + 1) > 128`: the rows' column and + /// sign fields would overlap in the one 128-bit key hash. pub fn with_dimensions_and_seed(rows: usize, cols: usize, seed_idx: usize) -> Self { + if let Err(problem) = check_l2hh_hash_bits(rows, cols) { + panic!("{problem}"); + } let mut sk = CountL2HH { counts: Vector2D::init(rows, cols), l2: Vector1D::filled(rows, 0), @@ -1186,6 +1216,21 @@ impl CountL2HH { mod tests_count_l2_hh { use super::*; + /// 11 rows x (11 column bits + 1 sign bit) = 132 bits of one 128-bit + /// hash: the deep rows' columns would reuse the shallow rows' signs. + #[test] + #[should_panic(expected = "hash bits")] + fn countl2hh_rejects_column_bits_that_reach_the_sign_bits() { + let _: CountL2HH = CountL2HH::with_dimensions_and_seed(11, 2048, 0); + } + + /// 10 x 2048 spends 120 bits and is accepted. + #[test] + fn countl2hh_accepts_the_widest_disjoint_geometry() { + let sketch: CountL2HH = CountL2HH::with_dimensions_and_seed(10, 2048, 0); + assert_eq!((sketch.rows(), sketch.cols()), (10, 2048)); + } + #[test] fn countl2hh_estimates_and_l2_are_consistent() { let mut sketch: CountL2HH = CountL2HH::with_dimensions(3, 32); diff --git a/src/sketches/countsketch_topk/l2hh_wire.rs b/src/sketches/countsketch_topk/l2hh_wire.rs index 4937e7b0..014a9f04 100644 --- a/src/sketches/countsketch_topk/l2hh_wire.rs +++ b/src/sketches/countsketch_topk/l2hh_wire.rs @@ -32,11 +32,10 @@ use serde::{Deserialize, Serialize}; use std::marker::PhantomData; use crate::common::hash::check_matrix_rows; -use crate::common::structures::matrix_storage::cols_mask_bits; use crate::message_pack_format::envelope; use crate::{HashProfile, SketchHasher, Vector1D, Vector2D}; -use super::CountL2HH; +use super::{CountL2HH, check_l2hh_hash_bits}; /// CountL2HH kind_id: family `0x19`, single algorithm variant `0x00`. const L2HH_KIND: &[u8] = &[0x19, 0x00]; @@ -96,13 +95,9 @@ struct L2hhPayload { /// `Vector2D` derives its column mask via `cols.ilog2()`, which panics on /// `cols == 0`. /// -/// CountL2HH draws one 128-bit hash per key and slices it per row: row `r` -/// reads its column from bits `[r * mask_bits, (r + 1) * mask_bits)` and its -/// sign from bit `127 - r`, where `mask_bits = cols_mask_bits(cols)`. Each row -/// therefore claims `mask_bits + 1` of the 128 bits, so the fields are in -/// range and disjoint exactly while `rows * (mask_bits + 1) <= 128` — the -/// budget `::assert_compatible` asserts, restated here -/// as a `Result` so both wire doors fail closed instead of panicking. +/// The hash-bit budget is [`check_l2hh_hash_bits`], the same check the +/// constructor panics on, returned here as a `Result` so both wire doors fail +/// closed instead of panicking. pub(crate) fn check_dimensions(rows: usize, cols: usize) -> Result<(), String> { if rows == 0 || cols == 0 { return Err(format!( @@ -110,15 +105,7 @@ pub(crate) fn check_dimensions(rows: usize, cols: usize) -> Result<(), String> { )); } check_matrix_rows("CountL2HH", rows)?; - let mask_bits = cols_mask_bits(cols) as usize; - let bits_required = mask_bits.saturating_add(1).saturating_mul(rows); - if bits_required > 128 { - return Err(format!( - "CountL2HH rows={rows}, cols={cols} needs {bits_required} hash bits \ - ({mask_bits} column bits + 1 sign bit per row), past the 128 the key hash carries" - )); - } - Ok(()) + check_l2hh_hash_bits(rows, cols) } /// Rejects a seed index the hasher's seed list does not hold. Seed selection @@ -593,6 +580,19 @@ mod tests { ); } + /// A sketch past the hash budget, built without the constructor's check, + /// as a struct deserialized by another route could be. + fn unchecked(rows: usize, cols: usize) -> Std { + CountL2HH { + counts: Vector2D::init(rows, cols), + l2: Vector1D::filled(rows, 0), + row: rows, + col: cols, + seed_idx: 0, + _hasher: PhantomData, + } + } + /// One 128-bit key hash gives each row `mask_bits + 1` bits, so a geometry /// needing more than 128 of them is not a sketch either door may pass. /// `rows = 12, cols = 4096` and `rows = 13, cols = 2048` both need 156: @@ -600,7 +600,7 @@ mod tests { #[test] fn count_l2hh_rejects_a_geometry_past_the_hash_budget() { for (rows, cols) in [(12usize, 4096usize), (13, 2048)] { - let problem = Std::with_dimensions(rows, cols) + let problem = unchecked(rows, cols) .serialize_to_bytes() .expect_err("a geometry past the hash budget must not serialize") .to_string(); @@ -624,14 +624,14 @@ mod tests { #[test] fn count_l2hh_rejects_column_fields_overlapping_the_sign_bits() { assert!( - Std::with_dimensions(10, 4096).serialize_to_bytes().is_err(), + unchecked(10, 4096).serialize_to_bytes().is_err(), "column fields must not reach into the sign bits" ); // The widest geometry each column count still admits. assert!(Std::with_dimensions(9, 4096).serialize_to_bytes().is_ok()); assert!(Std::with_dimensions(10, 2048).serialize_to_bytes().is_ok()); assert!( - Std::with_dimensions(11, 2048).serialize_to_bytes().is_err(), + unchecked(11, 2048).serialize_to_bytes().is_err(), "column fields must not reach into the sign bits" ); }