Skip to content

Fixed bug in experiment scripts, updated Utilities/rsync scripts to rsync files in repo root#5

Merged
milindsrivastava1997 merged 1 commit into
mainfrom
2-tracking-issue-for-changes-to-port-over-from-the-private-repo
Mar 3, 2026
Merged

Fixed bug in experiment scripts, updated Utilities/rsync scripts to rsync files in repo root#5
milindsrivastava1997 merged 1 commit into
mainfrom
2-tracking-issue-for-changes-to-port-over-from-the-private-repo

Conversation

@milindsrivastava1997

Copy link
Copy Markdown
Contributor

No description provided.

@milindsrivastava1997 milindsrivastava1997 linked an issue Mar 3, 2026 that may be closed by this pull request
@milindsrivastava1997 milindsrivastava1997 merged commit 1c25ac6 into main Mar 3, 2026
10 checks passed
@milindsrivastava1997 milindsrivastava1997 deleted the 2-tracking-issue-for-changes-to-port-over-from-the-private-repo branch March 3, 2026 01:01
GordonYuanyc added a commit that referenced this pull request Jun 2, 2026
…#5)

Resolves the half-applied abstraction the reviewer flagged: the module no
longer pretends to isolate the upstream sketch types. Accumulators now call
the concrete `RuntimeCountMin`/`RuntimeKll` API directly for trivial
operations, and `sketchlib_runtime` keeps only the logic worth sharing.

Inlined (pure single-call delegations) into the accumulators:
- cms_new      -> RuntimeCountMin::with_dimensions(..)
- cms_estimate -> self.inner.estimate(&DataInput::String(..))
- kll_new      -> RuntimeKll::init_kll(k as i32)
- kll_update   -> self.inner.update(&value)

Kept in sketchlib_runtime (real logic, not pass-throughs):
- wire codec: cms_/kll_ to/from_msgpack, cms_matrix, cms_from_matrix
- validated merge: cms_merge_refs, kll_merge_refs
- guarded ops shared by >1 caller / carrying an invariant: cms_update
  (non-negative guard), kll_quantile (empty-sketch default), kll_sketch_bytes

Module doc rewritten to state plainly that this is not an isolation layer,
so the direct `self.inner.rows()/.cols()/.k()` accesses are intentional
rather than a leak.

Co-Authored-By: Claude Opus 4.8 (1M context) <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.

Tracking issue for changes to port over from the private repo

1 participant