Skip to content

Measure accuracy after merging m shards - #131

Merged
milindsrivastava1997 merged 6 commits into
mainfrom
merged-accuracy
Oct 5, 2026
Merged

milindsrivastava1997 merged 6 commits into
mainfrom
merged-accuracy

Conversation

@zzylol

@zzylol zzylol commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Adds accuracy after merging to sketch-bench. Before this PR, --operations merge --metrics accuracy failed with "nothing measures the accuracy of merge, for any sketch".

  • --operations merge --metrics accuracy --merge-shards m: splits the N-item stream into m contiguous shards, builds one sketch per shard, and merges them into one. The merged sketch is scored with the same probes, comparator and full-stream ground truth as the single-sketch query, and reported as merge_accuracy. Sketches without merge fail with the registry's existing message.
  • scripts/study_saturation.py --merge-shards-list 1,4,16,64: writes saturation_merge_curve.csv, giving error vs N per shard count.
  • Tests: CMS merged accuracy equals single-sketch accuracy exactly; KLL merged accuracy is finite.

Findings

Bounded run: first config per sketch, N ≤ 1e7, 3 seeds, θ ∈ {0.5, 1.0, 1.5}, K ∈ {1e3, 1e5, 1e7}, α ∈ {1.1, 2}.

  • CMS, CountSketch, HLL, DDSketch: merged error is identical to the single sketch in every cell (0 of 1479 differ). Their saturation curves apply directly to merged windows.
  • KLL (k = 50): within seed noise up to 16 shards; 5–16% higher at 64 shards.
  • CMS-heap top-k (3×256): unchanged at K = 1e3. At K ≥ 1e5 and θ ≥ 1, precision@k drops by 0.03–0.09 (8–25%). Single-sketch curves overstate merged top-k quality.

Follow-up

The run covers one config per sketch. scripts/recommend_config.py (in #130) therefore still flags top-k/KLL recommendations without a matching merge curve as possibly optimistic.

Validation

  • Before the rebase: cargo fmt --check, cargo test for aqpbm-core/sketch-bench/aqpbm-cli, and cargo clippy --all-targets -D warnings all passed.
  • After the rebase: Python unittests pass, and the merge_accuracy integration test passes.

🤖 Generated with Claude Code


Update: full grids

Merged accuracy now covers:

  • all 8 CMS-heap top-k configs (3 seeds);
  • KLL k 50/200/800 and DDSketch α_dd 0.005–0.05 (10 seeds).

Results are in docs/figures/saturation/merge_grid/:

Sketch Merged vs single sketch
DDSketch exact everywhere
Top-k median precision unchanged; drops of up to 40% at large K, for every width
KLL merged error grows with k: ≈ 1.0–1.1× at k = 50/200, 1.12–1.32× at k = 800 at N = 1e7, and up to 3–4× at N = 1e4–1e5 (k = 800, m = 4)

recommend_config.py --merge-curves now has a merged curve for every top-k and KLL config:

  • No recommendation is flagged "single-sketch curve optimistic" any more; before this there were 43.
  • 6 recommendations changed. For example, p99 latency over 5m windows now needs KLL k = 200, not k = 50.

The math behind the KLL result is in #130's docs/saturation_conclusions.md.

*merge_supported,
"merge_supported"
);
keep_owned!(out.merge.accuracy, accuracy.clone(), "accuracy");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merge accuracy can't be flattened together with any other merge metric. Every merge record stamps merge_shards/merge_supported (main.rs:440, bench_report.rs:52), and keep_owned! on merge_shards (line 169) rejects the second value.

Repro on this branch:

--operations merge --metrics throughput,accuracy --flat
Error: flatten_record: cms-fastpath-vector2d/lib merge has two values for merge_shards

Same failure from approxbench flatten joining a cost pass (merge throughput) with an accuracy pass that now includes merge accuracy, which is the join flatten_cmd.rs exists for. Throughput+latency on merge already hit this before, but this PR makes it the common case. These two fields probably want keep_shared! (equal values allowed) rather than keep_owned!.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make sense

Comment thread scripts/study_saturation.py Outdated
curve_file.flush()
if shard_list:
merge_path = os.path.join(args.out, "saturation_merge_curve.csv")
append = args.resume and os.path.exists(merge_path)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With --resume, saturation_merge_curve.csv is only appended to, while saturation_curve.csv is rewritten with complete points only.

  • Duplicates: if a run is killed mid-point after the 8 KB buffer flushed some of that point's merge rows, those rows stay; the resumed run re-runs the point and appends them again. plot_saturation_merge.py then plots repeated (n, error) per m, garbling lines/bands and the ratio at max N.
  • Gaps: a point whose curve is complete but whose merge rows were never written (kill between curve_file.flush() and merge_file.flush(), or an earlier run without --merge-shards-list) is skipped and never gets merge rows.

Suggest rewriting the merge CSV on resume the same way as the curve CSV: keep only rows for points that are complete in both, and re-run points missing merge rows.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makse sense

ys = [p[1] for p in pts]
ses = [p[2] for p in pts]
ax.fill_between(ns, [y - 2 * s for y, s in zip(ys, ses)], [y + 2 * s for y, s in zip(ys, ses)],
color=SHARD_COLORS[m], alpha=0.12, lw=0)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SHARD_COLORS only has keys 1/4/16/64, so any other --merge-shards-list (e.g. 1,2,8) dies with KeyError here. The summary plot also hardcodes (4, 16, 64), so other shard counts would silently be dropped there even with a color. Suggest deriving the shard counts from the data and falling back to a palette cycle for unknown m.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense. you can extend.

@milindsrivastava1997
milindsrivastava1997 marked this pull request as ready for review October 4, 2026 21:09
@milindsrivastava1997
milindsrivastava1997 changed the base branch from study-saturation to saturation-study October 4, 2026 21:18
@milindsrivastava1997
milindsrivastava1997 changed the base branch from saturation-study to main October 4, 2026 21:26
zzylol and others added 6 commits October 4, 2026 17:49
With --merge-shards m, the stream is split into m contiguous shards, one
sketch is built per shard, the sketches are folded into one, and that sketch
is asked the query's probes and scored with the same comparator against the
whole-stream truth, as merge_accuracy. Every mergeable row supplies the
asked-after-fold body next to its merge and merge-step bodies; the plain
query is the one-shard case. Rows without merge keep the registry error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
For each point and N, also score the sketch merged from m contiguous shards
for every m>1 in the list, into saturation_merge_curve.csv (m=1 repeats the
single-sketch curve). Without the flag nothing changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the "Accuracy after merging" gap with results from a bounded
--merge-shards-list 1,4,16,64 run: linear sketches, HLL and DDSketch merge
exactly; KLL drifts within seed noise up to 64 shards; the top-k heap loses
precision once there are more keys than its heap holds.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Error vs N per shard count for every sketch and distribution, and a
merged/single ratio summary at the largest N, with a reading guide.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
All top-k configs and KLL k 50/200/800 and DDSketch with 10 seeds, merged
at 4/16/64 shards; plots per config, a merged/single summary, and the
recommendations rerun with these merge curves.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- flatten_record: merge_shards/merge_supported may repeat across merge
  squares when equal, so throughput+accuracy on merge flattens.
- study_saturation --resume rewrites saturation_merge_curve.csv with only
  complete points and reruns points whose merge curve is missing.
- plot_saturation_merge: colours and summary follow the shard counts in the
  data; exits when the m=1 baseline is missing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@GordonYuanyc GordonYuanyc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! LGTM! I'm convinced this is the right way to measure merge accuracy.

@milindsrivastava1997
milindsrivastava1997 merged commit bd644fe into main Oct 5, 2026
2 checks passed
zzylol added a commit that referenced this pull request Oct 5, 2026
#131 merged into main as bd644fe with content identical to the
merged-accuracy head the merge curves came from, so drop the
not-merged caveat. The table and its recorded revision are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Oct 5, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Oct 5, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Oct 5, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Oct 5, 2026
…#136)

* Export the AutoSketch vs ASAP evaluation table from saturation curves

Adds scripts/export_autosketch_eval_table.py and the generated
rqe-optimizer/data/autosketch-eval/table.json (W2: alibaba_v2022, boom,
google_2011). Every config lists AutoSketch's single-sketch error at
max_N and ASAP's error after merging m = 1/4/16/64 shards, with the
saturation flag ASAP requires. recommend_config.py's grid rounding moves
into grid_point() so both scripts share it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* Note that sketch-bench #131 has merged

#131 merged into main as bd644fe with content identical to the
merged-accuracy head the merge curves came from, so drop the
not-merged caveat. The table and its recorded revision are unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Oct 5, 2026
- rqe-optimizer/results/autosketch-vs-asap-synthetic: 66 runs of the
  ProjectASAP/ASAPQuery#777 workload grid (default point, every dimension
  alone, replicas x series, windows x card), model A and model B per family,
  every SLA. 0 sanity violations, the peak bound exact everywhere, no solve
  or job timed out. Raw results as sweep-json.tar.gz; sweep.csv, summary,
  figures and per-shard provenance (34 runs at ba52fd8 with verified
  identical tables, 32 at 0a45c48).
- rqe-optimizer/data/profiling-time.json: ASAP's one-time profiling, 35.6 h
  of sketch-bench runs (#130, #131, #140), from record timestamps.
- The synthetic summary and figures order each dimension's values by the
  grid.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

3 participants