Repository navigation
feat(countsketch)!: ASAPv1 codec with counter type and mode; drop the protobuf, msgpack and delta formats - #89
Draft
GordonYuanyc wants to merge 2 commits into
Draft
GordonYuanyc wants to merge 2 commits into
GordonYuanyc wants to merge 2 commits into
Conversation
… formats CountSketch gains CounterType (float64, i32, i64) and Mode (fast, regular). Update, UpdateWeight and Estimate follow the regular derivation when Mode is ModeRegular. MarshalASAPv1 / UnmarshalASAPv1 carry kind 0x04 0x00; a float64-counter sketch has no ASAPv1 encoding and fails to marshal. Removed: SerializePortable, SerializeProtoBytes, DeserializeCountSketchFromProtoBytes, SerializeMsgpack, DeserializeMsgpack, SerializeDelta, DeserializeDelta, the asapmsgpack plain, sparse-delta and cell-delta Count Sketch codecs, the CountSketchDelta / CountSketchCell protos and the count_sketch fields of both proto envelopes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
UpdateString, UpdateStringGOS, the sampled and pre-admitted row updates, EstimateStringCount, ProcessInput, ColForRow / SignForRow and the top-k rebuilds in Merge, ApplyDelta and the wire heap now derive rows from the key bytes under the sketch's Mode, using the key's matrix hash on the fast path for every geometry. A write from a precomputed hash alone that does not follow Mode marks the sketch, Merge carries the mark, Reset clears it, and MarshalASAPv1 refuses a marked sketch. Also: document the power-of-two cols limit on UnmarshalASAPv1, reserve the count_sketch name in both envelopes, and drop stale CountSketchDelta references from the Count-Min proto and delta docs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Oct 2, 2026
Draft
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.
Stacked on #85 (
asapv1-only/infra). Part of the ASAPv1-only series: Go's Count Sketch gets an ASAPv1 codec and loses its protobuf, msgpack and delta formats.Draft because of the open questions below.
Added
CounterType(CounterFloat64default,CounterInt32,CounterInt64) andMode(ModeFastdefault,ModeRegular) onCountSketch, andMarshalASAPv1/UnmarshalASAPv1(kind0x04 0x00). All threecs_*fixtures passCheckGolden.Update*,UpdateString*, the sampled/at-rows/GOS variants,Estimate*,ProcessInput/ColForRow/SignForRow/UpdateCellwith key bytes, and the top-k rebuilds) derives cells through one mode-aware path that matches Rust's regular path or Rust's fast path on every geometry (packed-64, packed-128, per-row). Confirmed byte-identical to Rust for both modes.Merge, cleared byReset) andMarshalASAPv1refuses.Changed
Updateand Rust on matrices outside the packed-64 layout (they used Go-onlymix64hashing there).Removed
SerializePortable,SerializeProtoBytes,DeserializeCountSketchFromProtoBytes,SerializeMsgpack/DeserializeMsgpack,SerializeDelta/DeserializeDelta; asapmsgpack CS full, cell-delta and sparse-delta codecs; protoCountSketchDelta,CountSketchCell, andcount_sketchin both envelopes (reserved 11; reserved "count_sketch";).CountSketchStatestays for UnivMon and Hydra (Hydra's cell now builds it locally).Open questions
CountSketchDelta; both are removed and ASAPv1 has no delta."f64"Count Sketch counter type.cols: Go decodes only power-of-two widths, so a valid Rust sketch with e.g.cols = 3is rejected (documented onUnmarshalASAPv1, tested with real Rust bytes).IncrCell,SetCell,MergeDelta) don't set the refusal flag; the OctoSketch per-cell path now hashes key bytes on every call (not benchmarked).Verification (Go 1.24.9)
go build,go vet,gofmt,go test ./.... Known failures also onmain: CocoSketch tests,TestKLL_Reset_SubsequentInserts(flaky),tests/cross_languagewithoutXTEST_DIR. Implemented and reviewed by separate agents; review findings are addressed in the second commit.🤖 Generated with Claude Code