phase3(parity): un-ignore cms byte-parity test + align hll fixture generator - #254
Merged
Merged
Conversation
…nerator ## 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>
zzylol
added a commit
that referenced
this pull request
May 5, 2026
Two updates rolled into one PR: ## PROGRESS.md — byte-parity completion (closes #243 follow-on) Adds a top-of-file dated section "Cross-language byte-format parity, 5/5 sketches (2026-05-05)" covering the chain of work that closed #243: - asap_sketchlib PRs #43 / #44 / #45 — HLL hash-seed alignment, CountSketch HashSpec / derive_index / derive_sign port (also extracts the shared `common::hashspec` module), CountMinSketch port routing through the same primitives. - ASAPCollector PR #254 — un-ignores `cms_byte_parity_with_go` with the matching FO-mode wrapper rewrite, and fixes a latent inconsistency in the HLL fixture generator that #252 missed (HLL was using `SerializeProtoBytes()` directly while every other sketch goes through `SerializePortable*` + strip + Marshal; the test only "passed" because fixtures are gitignored and rarely regenerated alongside a test run). - Verification: 7/7 cross_language_parity tests pass with `--include-ignored`, including all 5 byte-parity tests + 2 sanity tests, no ignored remaining. Also flags the downstream unblock: `ASAPQuery-backend`'s `edge_runtime_consumes_precompute_rs.rs` HLL / CS / CMS round-trip tests were `#[ignore = "blocked on ASAPCollector#243"]` per `design-phase3-asap-precompute-rs.md` — they should now pass without backend code changes (mechanical un-ignore PR pending, separate work). ## docs/paper-outline.md — five-claim eval framing The "Measurable benefits at every layer" bullet was 4 lines listing benefits without pointing at *how* the paper proves each one. Replaces it with five named evaluation dimensions (transmission bandwidth, edge CPU, edge memory, query accuracy, query latency) plus the combined Pareto headline claim, with explicit evidence-tooling pointers per claim: - bandwidth: `run_e2e_sweep.sh` (P7) bytes columns + `e2e_plots.py` (P9) bandwidth-vs-N + `cardinality_crossover/` single-host pre-compare. - edge CPU: P7 producer-side cpu column + `bench_2node_sim.sh` + the SDK label-axis profile (paper blocker #2). - edge memory: P7 rss column + `bench_soak.sh` (steady-state RSS / heap / fd-count + slope-based leak verdict). - accuracy: `raw_tee.go` (P4) ground truth + `accuracy_reduce.py` (P8) join + `ASAPQuery-backend/TODO.md` "Accuracy-profile library per sketch type" + `sketchlib-bench/docs/DESIGN.md` for bound derivation. - query latency: `promql_replay.py` (P5) p50/p99 + P9 `query_latency_cdf.png` + `plan_transition.py` (P6) transition timings. The combined Pareto claim (`pareto_acc_vs_thru.png` from P9) is named explicitly as the figure the paper's contribution rests on. The "Formal correctness" bullet stays unchanged. Architectural sibling — the existing [`design-asap-edge-framework.md`](design-asap-edge-framework.md) — covers the system shape; this rewrite makes the empirical bar for "done" explicit alongside it. 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
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/::estimatethrough the sharedasap_sketchlib::common::hashspecpipeline. With the upstream fix landed, the matrix layout matches Go cell-for-cell. The remaining gap was wrapper-side: the Rust wrapper was emitting aCountMinStatewithcounter_type = FLOAT64+counts_float, while Go'sCountMinSketch.SerializeProtoBytesFOemits the Frequency-Only payload withcounter_type = INT64+counts_int+ per-rowl1/l2norms.asap-precompute-rs/src/sketches/cms.rs:CMSWrapper::build_statenow emits the FO payload —counter_type=INT64, packed sint64counts_int, per-rowl1[r] = sum_c counts[r][c]andl2[r] = sum_c counts[r][c]^2.sum_counts/sum2_countsremain empty (Frequency-Only mode).asap-precompute-rs/tests/cross_language_parity.rs: drop the#[ignore]reason citing the pre-distributed SDKs -> 1 collector, distributed collectors #45 hash-layer divergence.integration/parity/golden_test.go: switch the CMS fixture generator fromsk.SerializeProtoBytesFO()tosk.SerializePortableFO()+ stripProducer/HashSpec+proto.Marshal(matches the DDSketch / KLL / CountSketch pattern from PRs phase3(parity): un-ignore ddsketch byte-parity test #247 / phase3(parity): un-ignore kll byte-parity test, route through wire_* accessors #250 / phase3(parity): un-ignore countsketch byte-parity test, route wrapper through INT64/L2 #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 toNone. PR #252 un-ignoredhll_byte_parity_with_gobut did not touchgolden_test.go, so a regenerated fixture diverged from Rust by exactly the 134-byte Producer + HashSpec 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 usessk.SerializePortable()+ stripProducer/HashSpec+proto.Marshal, matching DDSketch / KLL / CountSketch / CMS.Verification
GOLDEN_REGEN=1 go test -run TestGenerateGoldenFixtures ./integration/parity/...— passes; HLL fixture goes 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.
🤖 Generated with Claude Code