fix(portable): lay out the CountSketch row hash by shape

CHANGELOG.md

@@ -92,6 +92,14 @@ signals a backwards-compatible change.
 
 ### Fixed
 
+- **The portable `CountSketch` hashes every row independently at any width.**
+  It sliced every row's column and sign from one 64-bit hash whatever the
+  shape, so once `rows * (ceil_log2(cols) + 1)` passed 64, deep rows reused
+  shallow rows' bits (at `30000` columns only about four rows were
+  independent) and debug builds panicked on the shift. It now lays the hash
+  out by shape as the native fast-path sketches do: one 64-bit hash while the
+  rows fit, then one 128-bit hash, then one hash per row. Shapes that fit 64
+  bits keep their cells, so the Go parity probe is unchanged.
 - `KLLDynamic` panicked on every quantile query once a NaN had entered the
   stream: the CDF was sorted with `partial_cmp(..).unwrap()`. It sorts with
   `total_cmp`, as the three sibling sorts beside it already did.

docs/tests.md

@@ -1260,6 +1260,8 @@ Test file: [`src/message_pack_format/portable/countsketch.rs`](../src/message_pa
 
 | test_name | test_description | what_is_tested |
 | --- | --- | --- |
+| `deep_shapes_hash_every_row_independently` | A shape too deep for one 64-bit hash hashes every row independently and does not panic. | For a `15x30000` sketch (15 column bits and a sign bit per row), verifies one update of `2.5` reads back `2.5`, and that over 200 keys no two rows put more than 2 keys in the same column. |
+| `packed_shapes_keep_their_cells` | A shape that fits one 64-bit hash keeps the Go-parity cells. | For a `5x1024` sketch, verifies each row's column for `"api"` is the column the packed 64-bit hash at seed index `0` gives that row. |
 | `msgpack_delta_against_empty_round_trips` | A msgpack delta against an empty snapshot carries the signed cells exactly. | For a `5x1024` sketch fed 200 updates of `"hot"` at weight `1.0`, verifies the delta against a fresh sketch at threshold `1.0`, applied to another fresh sketch, reproduces `sketch()` cell for cell. |
 | `msgpack_delta_wire_layout_is_parallel_arrays` | The delta is a five-element array of scalars plus parallel index arrays. | For a `3x8` sketch fed `"x"` at weight `5.0`, verifies the delta bytes equal `rmp_serde::to_vec(&(rows as u64, cols as u64, row_idx: Vec<u32>, col_idx: Vec<u32>, d_count: Vec<i64>))` recomputed from the non-zero cells, and that decoding reports `rows` `3` and `cols` `8`. |
 | `test_new_empty` | A fresh sketch has the requested shape and zeroed cells. | For `CountSketch::new(2, 3)`, verifies `rows` is `2`, `cols` is `3`, and `sketch()` is `[[0.0, 0.0, 0.0], [0.0, 0.0, 0.0]]`. |

src/message_pack_format/portable/countsketch.rs

@@ -148,24 +148,29 @@ impl CountSketch {
         }
     }
 
-    /// Insert a single weighted observation. Uses the shared
-    /// [`crate::MatrixHashType::Packed64`] fast-path hash so the matrix-cell
-    /// layout matches `sketchlib-go::CountSketch.UpdateString`
-    /// bit-for-bit. Locked in by
-    /// `tests/sketches_go_parity_probe.rs`.
-    ///
-    /// Constraints inherited from Go's Packed64 mode:
-    /// - `cols` must be a power of two; the column mask is `cols - 1`.
-    /// - `rows * (log2(cols) + 1) ≤ 64` so the packed hash holds all
-    ///   row indices and sign bits.
+    /// The key's per-row hash, laid out for this shape as the native
+    /// fast-path sketches lay it out: one 64-bit hash while every row's
+    /// column bits and sign bit fit in it, then one 128-bit hash, then one
+    /// hash per row. Packed rows therefore never share hash bits, and a shape
+    /// that fits 64 bits keeps the cells `sketchlib-go::CountSketch.UpdateString`
+    /// gives it (locked in by `tests/sketches_go_parity_probe.rs`). One hash
+    /// per row draws row `r`'s seed from the seed list, so past
+    /// [`crate::MATRIX_MAX_ROWS`] rows, row `r + MATRIX_MAX_ROWS` repeats row `r`.
+    fn row_hash(&self, key: &str) -> crate::common::MatrixHashType {
+        crate::common::hash_for_matrix_seeded(
+            0,
+            self.rows,
+            self.cols,
+            &crate::common::DataInput::String(key.to_owned()),
+        )
+    }
+
+    /// Insert a single weighted observation, hashed as [`Self::row_hash`] says.
     pub fn update(&mut self, key: &str, value: f64) {
         if self.rows == 0 || self.cols == 0 {
             return;
         }
-        let input = crate::common::DataInput::String(key.to_owned());
-        let hashed = crate::common::MatrixHashType::Packed64(
-            <crate::DefaultXxHasher as crate::common::SketchHasher>::hash64_seeded(0, &input),
-        );
+        let hashed = self.row_hash(key);
         for r in 0..self.rows {
             let col = <crate::common::MatrixHashType as crate::common::MatrixFastHash>::col_for_row(
                 &hashed, r, self.cols,
@@ -185,10 +190,7 @@ impl CountSketch {
         if self.rows == 0 || self.cols == 0 {
             return 0.0;
         }
-        let input = crate::common::DataInput::String(key.to_owned());
-        let hashed = crate::common::MatrixHashType::Packed64(
-            <crate::DefaultXxHasher as crate::common::SketchHasher>::hash64_seeded(0, &input),
-        );
+        let hashed = self.row_hash(key);
         let mut estimates: Vec<f64> = Vec::with_capacity(self.rows);
         for r in 0..self.rows {
             let col = <crate::common::MatrixHashType as crate::common::MatrixFastHash>::col_for_row(
@@ -541,6 +543,57 @@ impl MessagePackCodec for CountSketchDelta {
 mod tests {
     use super::*;
 
+    /// The column each row gives `key`, read off a fresh sketch.
+    fn columns(rows: usize, cols: usize, key: &str) -> Vec<usize> {
+        let mut sketch = CountSketch::new(rows, cols);
+        sketch.update(key, 1.0);
+        sketch
+            .matrix
+            .iter()
+            .map(|row| row.iter().position(|cell| *cell != 0.0).unwrap())
+            .collect()
+    }
+
+    /// A shape too deep for one 64-bit hash (15 rows of 15 column bits and a
+    /// sign) reads back its weight, and its rows place keys independently:
+    /// two rows rarely put a key in the same column.
+    #[test]
+    fn deep_shapes_hash_every_row_independently() {
+        let (rows, cols) = (15, 30_000);
+        let mut sketch = CountSketch::new(rows, cols);
+        sketch.update("api", 2.5);
+        assert_eq!(sketch.estimate("api"), 2.5);
+        let placed: Vec<Vec<usize>> = (0..200)
+            .map(|i| columns(rows, cols, &format!("key-{i}")))
+            .collect();
+        for a in 0..rows {
+            for b in a + 1..rows {
+                let shared = placed.iter().filter(|c| c[a] == c[b]).count();
+                // Expected 200 / 30000 < 1 per pair.
+                assert!(shared <= 2, "rows {a} and {b} share {shared} columns");
+            }
+        }
+    }
+
+    /// A shape that fits one 64-bit hash keeps its cells: the layout the Go
+    /// parity probe pins does not move.
+    #[test]
+    fn packed_shapes_keep_their_cells() {
+        let hash = <crate::DefaultXxHasher as crate::common::SketchHasher>::hash64_seeded(
+            0,
+            &crate::common::DataInput::String("api".to_owned()),
+        );
+        let packed = crate::common::MatrixHashType::Packed64(hash);
+        let expected: Vec<usize> = (0..5)
+            .map(|row| {
+                <crate::common::MatrixHashType as crate::common::MatrixFastHash>::col_for_row(
+                    &packed, row, 1024,
+                )
+            })
+            .collect();
+        assert_eq!(columns(5, 1024, "api"), expected);
+    }
+
     #[test]
     fn msgpack_delta_against_empty_round_trips() {
         let (rows, cols) = (5, 1024);