Skip to content

fix(precompute): count frequency sketch events - #481

Merged
zzylol merged 1 commit into
mainfrom
fix/frequency-sketch-unit-weight
Sep 3, 2026
Merged

zzylol merged 1 commit into
mainfrom
fix/frequency-sketch-unit-weight

Conversation

@zzylol

@zzylol zzylol commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Before

Bare CMS and CountSketch are the backend realizations for canonical SketchQuery::PointCount, but raw precompute added each datapoint scalar as the sketch weight. Five observations with values [1000, -7, 42.5, 0, 9999] therefore produced a value sum instead of frequency 5. ASAPQuery fixed the CMS count-vs-sum distinction in ProjectASAP/ASAPQuery#665.

After

  • Bare CMS and bare CountSketch add exactly one occurrence per accepted observation.
  • Value-weighted and count-weighted top-k remain explicit modes of the separate heap updaters.
  • Regression tests use non-unit and negative values so the old behavior cannot pass accidentally.

This adapts the upstream fix to the current canonical Planner model: bare frequency sketches have PointCount semantics, while exact/value sums are different summary families.

Verification

  • cargo fmt -p data_plane -- --check
  • cargo test -p data_plane precompute_engine::accumulator_factory::tests::bare_ (2 passed)
  • cargo test -p data_plane precompute_engine:: (259 passed)

The first local build attempt exhausted the worktree filesystem during linking; after removing only regenerated worktree target/ artifacts and using the repository shared target, the commands above passed.

@zzylol
zzylol merged commit d4e1988 into main Sep 3, 2026
1 check passed
@zzylol
zzylol deleted the fix/frequency-sketch-unit-weight branch September 3, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant