phase3(parity): un-ignore countsketch byte-parity test, route wrapper through INT64/L2 - #253
Merged
Merged
Conversation
… through INT64/L2 Pairs with asap_sketchlib PR #44, which ports `sketchlib-go::common.HashSpec` / `DeriveIndex` / `DeriveSign` into a new public module `asap_sketchlib::common::hashspec` and refactors `CountSketch::update` to feed the matrix through the same packed-`u64` hash-once → bit-slice-rows pipeline that Go uses. With that upstream fix landed, the matrix layout matches Go cell-for-cell and the remaining gap is wrapper-side: the Rust wrapper was emitting a `CountSketchState` with `counter_type = FLOAT64` + `counts_float`, while Go's `CountSketch.SerializePortable` emits `counter_type = INT64` + `counts_int` + per-row `l2`. This commit: - Updates `CountSketchWrapper::build_state` to mirror Go's `SerializePortable`: emit packed sint64 `counts_int` (Opt-2: 4–8× smaller than f64 for typical small-integer counter values) and per-row L2 norms derived as `l2[r] = sum_c counts[r][c]^2`. Both fields are required for byte parity; without them the envelope diverges in `counter_type`, `counts_*`, and `l2` simultaneously. - Updates `integration/parity/golden_test.go::TestGenerateGoldenFixtures` → `t.Run("CountSketch", …)` to strip `Producer` / `HashSpec` metadata before marshalling (matching the DDSketch / KLL / HLL cases) and to clear `HhKeys` (the Rust wrapper does not maintain a Space-Saving candidate tracker, so this list cannot be reproduced; downstream rebuilds TopK from the merged matrix anyway). - Removes the `#[ignore]` from `cross_language_parity::countsketch_byte_parity_with_go`. Regenerate the golden fixture with `GOLDEN_REGEN=1 go test -run TestGenerateGoldenFixtures ./integration/parity/...` and the test passes byte-equality against `integration/parity/golden/countsketch_envelope.bin`. Closes part of #243. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
zzylol
added a commit
that referenced
this pull request
May 5, 2026
…nerator (#254) ## Summary Closes the last byte-parity gap from #243 and fixes a latent inconsistency in the HLL fixture generator that PR #252 (HLL un-ignore) missed. ## CMS un-ignore (the original Phase 3 step 3 follow-up) Pairs with asap_sketchlib PR #45, which ports the wire-format `CountMinSketch::update` / `::estimate` through the shared `asap_sketchlib::common::hashspec` pipeline (`HashSpec::default()` → single XXH3-64-with-seed → per-row `derive_index` over a power-of-two-rounded column mask). With that upstream fix landed the matrix layout now matches Go cell-for-cell. The remaining gap was wrapper-side: the Rust wrapper was emitting a `CountMinState` with `counter_type = FLOAT64` + `counts_float`, while Go's `CountMinSketch.SerializeProtoBytesFO` emits the Frequency-Only payload with `counter_type = INT64` + `counts_int` + per-row `l1` / `l2` norms (the `Sum_*` mode is reserved for weighted streams). - `asap-precompute-rs/src/sketches/cms.rs`: `CMSWrapper::build_state` now emits the FO payload — `counter_type=INT64`, packed sint64 `counts_int`, per-row `l1[r] = sum_c counts[r][c]` and `l2[r] = sum_c counts[r][c]^2` (collapsed forms of Go's `InsertWithHash`-maintained running norms for the unweighted unit-step stream the parity harness drives — the only producer pattern this wire path serves today). `sum_counts` / `sum2_counts` remain empty (Frequency-Only mode). - `asap-precompute-rs/tests/cross_language_parity.rs`: drop the `#[ignore]` reason citing the pre-#45 hash-layer divergence. - `integration/parity/golden_test.go`: switch the CMS fixture generator from `sk.SerializeProtoBytesFO()` to `sk.SerializePortableFO()` + strip `Producer` / `HashSpec` + `proto.Marshal` (matches the DDSketch / KLL / CountSketch pattern established by PRs #247 / #250 / #253). ## HLL fixture generator alignment Discovered while regenerating fixtures end-to-end after the CMS work: the HLL fixture generator was the odd one out — it called `sk.SerializeProtoBytes()` directly (which embeds Producer + HashSpec), while the Rust HLL wrapper sets both fields to `None`. PR #252 un-ignored `hll_byte_parity_with_go` but did not touch `golden_test.go`, so the regenerated fixture diverged from Rust by exactly the 134-byte Producer + HashSpec metadata footprint. The test only "passed" because fixtures are gitignored and developers rarely regenerate-then-test in one step — once you do (`GOLDEN_REGEN=1 go test ... && cargo test --include-ignored ...`), HLL fails. - `integration/parity/golden_test.go`: HLL block now uses `sk.SerializePortable()` + strip `Producer` / `HashSpec` + `proto.Marshal`, matching DDSketch / KLL / CountSketch / CMS. ## Verification - `GOLDEN_REGEN=1 go test -run TestGenerateGoldenFixtures ./integration/parity/...` — passes; HLL fixture goes from 16532 → 16398 bytes (-134 = stripped Producer + HashSpec); CMS fixture is 8275 bytes (Frequency-Only INT64 payload). - `cargo test --release --test cross_language_parity -- --include-ignored` — 7/7 pass: ddsketch, kll, hll, countsketch, cms byte-parity tests + 2 sanity tests, no ignored remaining. - `cargo test --release -p asap-precompute-rs` — full suite green (27 lib tests + 7 parity tests). Closes #243. Co-authored-by: Claude Opus 4.7 (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
1 participant
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.
Summary
Pairs with asap_sketchlib PR #44, which ports
sketchlib-go::common.HashSpec/DeriveIndex/DeriveSigninto a new public moduleasap_sketchlib::common::hashspecand refactorsCountSketch::updateto feed the matrix through the same packed-u64hash-once → bit-slice-rows pipeline Go uses. With that upstream fix landed the matrix layout matches Go cell-for-cell. The remaining gap is wrapper-side: the Rust wrapper was emitting aCountSketchStatewithcounter_type = FLOAT64+counts_float, while Go'sCountSketch.SerializePortableemitscounter_type = INT64+counts_int+ per-rowl2.Changes
asap-precompute-rs/src/sketches/countsketch.rs:CountSketchWrapper::build_statenow mirrors Go'sSerializePortable— emit packed sint64counts_int(Opt-2 in Go, 4–8× smaller than f64 for typical counter values) and per-row L2 norms (l2[r] = sum_c counts[r][c]^2). Both fields are required for byte parity; without them the envelope diverges incounter_type,counts_*, andl2simultaneously.integration/parity/golden_test.go: the CountSketch fixture generator now stripsProducer/HashSpec(matching the DDSketch / KLL / HLL cases) and clearsHhKeys(the Rust wrapper does not maintain a Space-Saving candidate tracker, so this list cannot be reproduced; downstream rebuilds TopK from the merged matrix anyway).asap-precompute-rs/tests/cross_language_parity.rs: removes the#[ignore]oncountsketch_byte_parity_with_go.Test plan
cargo test --test cross_language_parity countsketch_byte_parity_with_go(against asap_sketchlib PR feat: memory pools in otel collector processors #44 branch) passes byte-equality against the regenerated golden.cargo testforasap-precompute-rspasses (43 + 11 + 5 + 27 tests; 2 unrelated KLL / CMS parity tests still ignored pending their own PRs).cargo clippy --all-targets -- -D warningsclean.integration/parity/golden/countsketch_envelope.bin(1577 bytes; gitignored) viaGOLDEN_REGEN=1 go test -run TestGenerateGoldenFixtures/CountSketch ./integration/parity/....origin/main.Closes part of #243.
🤖 Generated with Claude Code