Skip to content

Commit 97b192b

Browse files
GordonYuanycclaudezzylol
authored
fix: read sketch capabilities and state bytes from asap_sketchlib (#953, #946, #947) (#989)
* refactor(optimizer): derive Pass 1's summary candidates from the family-capability table `summary_candidates` was a hand-written intent → family list that repeated the family-capability table in the other direction (#953). It now takes the intent's statistic and keeps, in `FAMILY_PREFERENCE` order, the families whose table row answers it, less `UNIMPLEMENTED_FAMILIES` (Theta) and less UnivMon for a distinct-tuple count. Candidate lists and their order are unchanged. - The intent → statistic mapping is written once, `sketch_statistic`; Pass 1's `statistic` wraps it with its existing error. - `local_guarantee` returns `None` for a sketch read for a statistic its table row does not answer. Plain values and exact aggregates are not checked: the table lists no readouts for them. - Two estimator tests read CMS for a distinct count and Count Sketch for a total, which the table does not answer; they now read a statistic each family answers. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * build: pin asap_sketchlib to main 950bb5c main after release 0.4.0 carries #245–#248, which the old pin took from a branch, plus #249 (`memory_bytes` / `estimated_memory_bytes` across the sketches; KMV builds without `experimental`) and #251 (the public capability table). The executor drops the `experimental` feature: KMV was the only gated type it used. #251 makes `Kll::merge` panic when k or m differ. The KLL kernel checked only k, so a state adopted with another m would reach the panic; it now checks both and returns an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(types): read the sketch rows of the family-capability table from asap_sketchlib (#953) The family-capability table kept its own copy of what each sketch family answers and how it merges. A sketch family's row is now asap_sketchlib's (`SketchFamily::capability`, #251), mapped from `SketchAlgorithm` and `HydraKind` and read in the planner's vocabulary at query time: the seven statistics `StatisticKind` names, `NoGuarantee` as `Approximate`. The planner keeps the rows the library has none for: the exact accumulators and Theta. `readouts` becomes a method; its readers follow. Cells that change: - Count-Min and UnivMon totals are `Exact`, not `AsIfBuiltOverUnion`. Every consumer accepts both alike. - HydraKll merges disjoint inputs, its quantile as if built over the union: sketchlib's Hydra merge now requires the same k and m and returns an error otherwise. Nothing builds a HydraKll yet: Pass 1 offers no Hydra and the executor binds only HydraCms. - UnivMon lists its readouts in sketchlib's order, so its entry in the executor's supported summaries does too. sketchlib also lists DDSketch's total, the heap sketches' item frequency and HydraCms's total, which nothing here reads yet. `NOT_READ_YET` keeps them out, so Pass 1's candidates, the IR's estimate validation and the executor's supported summaries are unchanged; a test checks sketchlib still lists each entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(executor): charge DDSketch and UnivMon states sketchlib's byte counts (#947) A DDSketch state was reserved at the trait's flat 4096 bytes whatever it held, and its updater counted the bins in use; a UnivMon state by a hand formula (16 bytes per counter, 256 per heap slot). Both now read asap_sketchlib's count (#249): - DDSketch: `memory_bytes()`, the bins its stores allocate; the updater charges the same. - UnivMon: its own fields, `UnivMon::estimated_memory_bytes` for its shape with keys that own no bytes, and the existing bound on the variable-length keys its heaps retain. `memory_bytes()` walks every heap, and a build charges memory after every row, so the charge stays constant time. KLL, KMV, Count-Min, Count Sketch (with or without a heap) and HLL already charge what their sketch holds plus their own fields; Hydra has no sketchlib count. They are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(plan-selection): price summary state bytes with sketchlib's estimates (#946, #947) Plan selection priced Count-Min, Count Sketch (with or without a heap), DDSketch, HLL, KMV and UnivMon states from hand-copied sketchlib internals: 24 bytes per top-k heap slot (a slot holds 150–225 plus its key), 8 per UnivMon counter, DDSketch's bucket chunks. Each is now asap_sketchlib's `estimated_memory_bytes` (#249) for the type the executor builds (`f64` counters, `Vector2D` storage), given what `sketch_shape` assumes the state holds: - Count-Min, Count Sketch, HLL: exact. - KMV: the rows the state folds as its distinct items; exact only at that many distinct inputs, an upper bound below. - DDSketch: sketchlib's worst case over insertion orders and merges, for the rows it folds over the assumed nine decades, with negative values unless proven non-negative: 37,360 bytes at α = 0.01 with rows unknown, where the old model gave 18,576. - Count-Min / Count Sketch with heap: a full heap of keys of `ASSUMED_HEAP_KEY_BYTES`, an assumption of three 16-byte labels in the executor's tuple-key encoding (63 bytes). It applies to these heaps only. - UnivMon: every layer's heap full of keys of `ASSUMED_VALUE_KEY_BYTES`, one 16-byte label value with the executor's type tag (17 bytes): Pass 1 feeds UnivMon one column, which a distinct count over a label makes Utf8. KLL keeps the planner's own formula, unchanged: sketchlib's estimate also counts a CDF cache the executor never builds. Theta keeps its `k` hashes. An α outside (0, 1), where sketchlib's DDSketch estimate panics, is priced as an empty sketch. A test builds each family the executor builds per group through its factory, feeds it rows shaped as the pricing assumes, and compares the executor's charge with the priced bytes plus each accumulator's own named fields. The heap families' shapes are dominated by their heaps. - Count-Min, Count Sketch, HLL and KMV: equal. - KLL: equal up to k rows. Past k only a loose guard holds, at most sketchlib's 2 (k - 1) items of compaction scratch over the price; the formula can price below the charge there, a known gap until a sketchlib estimate without the CDF cache can price KLL. - DDSketch, the heap sketches and UnivMon: at least the charge, and at most 1.5 times it once the state is full. UnivMon's price also bounds the bytes its sketch holds, by at most 1.6 times: its estimate includes digest-index growth under churn that the test's labels do not reach. HydraCms is left out: plan selection prices an independent Count-Min per group, the executor one shared grid. Plan selection's tests gain the executor as a dev-dependency for the test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor: read asap_sketchlib's capability vocabulary directly; one executor support declaration - SketchAlgorithm, StatisticKind and MergedError are asap_sketchlib's (exhaustive, serde); the Planner's copies, the Readouts enum, NOT_READ_YET and the conversion matches are gone. FamilyCapability's sketch rows are the library's rows, unchanged; the exact accumulators keep Planner rows. - Theta is deleted (no library sketch), with UNIMPLEMENTED_FAMILIES. - The executor declares what it evaluates in one positive list, EVALUATED_READOUTS, replacing EVALUATED_FAMILIES and UNEVALUATED_READOUTS; a test checks every entry is in the table, so a sketchlib bump cannot widen the declared support. - The four hard-coded heap-family TopK checks read answers(TopK); the pcc.rs statistic() wrapper is gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: drop deleted Theta from KMV docs; name sketchlib #254 in the pin comment Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: zzylol <zeyingz@umd.edu> Co-authored-by: zzylol <50204836+zzylol@users.noreply.github.com>
1 parent 67d35f4 commit 97b192b

40 files changed

Lines changed: 975 additions & 654 deletions

‎Cargo.lock‎

Lines changed: 3 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎Cargo.toml‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@ resolver = "2"
2323
arrow = { version = "58.4", default-features = false }
2424
# The hasher Arrow already uses, for the executor's hot interning maps.
2525
ahash = { version = "0.8", default-features = false, features = ["std", "runtime-rng"] }
26-
# `planner-pin/0.4.0-plus`: release 0.4.0 (eb04e7f) plus the open sketchlib
27-
# PRs #245 (checked f64 updates, heap rescoring), #246 (KLL lazy-buffer
28-
# follow-ups), #247 (Count Sketch stack medians) and #248
29-
# (`DdSketch::quantile_interpolated`). Moves to a release once they merge.
30-
asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", rev = "63e565506426020c505ef0b84e0c91f450d09e7c" }
26+
# main after release 0.4.0 (eb04e7f): #245 (checked f64 updates, heap
27+
# rescoring), #246 (KLL lazy-buffer follow-ups), #247 (Count Sketch stack
28+
# medians), #248 (`DdSketch::quantile_interpolated`), #249 (`memory_bytes` and
29+
# `estimated_memory_bytes`; KMV without `experimental`), #251 (the public
30+
# capability table) and #254 (Planner names in that table). Moves to 0.4.1
31+
# once that is released.
32+
asap_sketchlib = { git = "https://github.com/ProjectASAP/asap_sketchlib", rev = "21b63578c2e612e153f5aaa4d3848f1035a13c60" }

‎crates/executor/Cargo.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ ahash.workspace = true
99
chrono = { version = "0.4.39", default-features = false, features = ["std"] }
1010
futures = "0.3"
1111
planner-types = { package = "asap-types", path = "../types" }
12-
asap_sketchlib = { workspace = true, features = ["experimental"] }
12+
asap_sketchlib = { workspace = true }
1313
serde = { version = "1", features = ["derive", "rc"] }
1414
serde_json = "1"
1515
thiserror = "2"

‎crates/executor/src/capability.rs‎

Lines changed: 27 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -325,37 +325,41 @@ pub fn validate_native_family(family: &SummaryFamilyType) -> Result<(), Error> {
325325
.map_err(Error::Invalid)
326326
}
327327

328-
/// The families the executor builds per instance and reads, in
329-
/// `SketchAlgorithm` order. A family it builds but reads nothing from
330-
/// (plain Count Sketch) and one it does not build (Theta) are absent.
331-
pub(crate) const EVALUATED_FAMILIES: [SketchAlgorithm; 8] = [
332-
SketchAlgorithm::UnivMon,
333-
SketchAlgorithm::Kll,
334-
SketchAlgorithm::Cms,
335-
SketchAlgorithm::Hll,
336-
SketchAlgorithm::DDSketch,
337-
SketchAlgorithm::CmsWithHeap,
338-
SketchAlgorithm::Kmv,
339-
SketchAlgorithm::CountSketchWithHeap,
328+
/// What the executor evaluates from a per-instance state of each family it
329+
/// builds and reads: its supported-summary declaration. Every readout here is
330+
/// in the family's row of the family-capability table; a readout the row
331+
/// lists and this omits is not evaluated yet (Count-Min's and the heap
332+
/// families' item frequency, which the executor answers only from a Hydra
333+
/// grid; KLL's rank; UnivMon's G-sum). Plain Count Sketch is built but read
334+
/// from nothing.
335+
pub(crate) const EVALUATED_READOUTS: &[(SketchAlgorithm, &[StatisticKind])] = &[
336+
(SketchAlgorithm::Cms, &[StatisticKind::Total]),
337+
(SketchAlgorithm::CmsWithHeap, &[StatisticKind::TopK]),
338+
(SketchAlgorithm::CountSketchWithHeap, &[StatisticKind::TopK]),
339+
(SketchAlgorithm::Hll, &[StatisticKind::Distinct]),
340+
(SketchAlgorithm::Kmv, &[StatisticKind::Distinct]),
341+
(SketchAlgorithm::Kll, &[StatisticKind::Quantile]),
342+
(SketchAlgorithm::DDSketch, &[StatisticKind::Quantile]),
343+
(
344+
SketchAlgorithm::UnivMon,
345+
&[
346+
StatisticKind::Total,
347+
StatisticKind::FrequencyL2,
348+
StatisticKind::Distinct,
349+
StatisticKind::FrequencyEntropy,
350+
],
351+
),
340352
];
341353

342-
/// The readouts the family-capability table lists for an evaluated family
343-
/// that the executor does not evaluate yet: a per-instance Count-Min's item
344-
/// frequency (the executor answers an item's frequency only from a Hydra
345-
/// grid, which Stage 2 does not produce yet; `SupportedSummary` gains a
346-
/// grouping dimension when it does).
347-
pub(crate) const UNEVALUATED_READOUTS: [(SketchAlgorithm, StatisticKind); 1] =
348-
[(SketchAlgorithm::Cms, StatisticKind::ItemFrequency)];
349-
350354
/// Whether the executor reads `readout` from a state of `family` whose
351355
/// family-capability table row answers it.
352356
fn evaluates(family: &SummaryFamilyType, readout: StatisticKind) -> bool {
353357
use planner_types::ir::schema::HydraKind;
354358
match family {
355359
SummaryFamilyType::Sketch(kind, GroupingStrategy::PerSubpopulationInstance) => {
356-
let algorithm = kind.algorithm();
357-
EVALUATED_FAMILIES.contains(algorithm)
358-
&& !UNEVALUATED_READOUTS.contains(&(algorithm.clone(), readout))
360+
EVALUATED_READOUTS
361+
.iter()
362+
.any(|(family, readouts)| family == kind.algorithm() && readouts.contains(&readout))
359363
}
360364
SummaryFamilyType::Sketch(_, GroupingStrategy::SharedMultiSubpopulation { kind, .. }) => {
361365
matches!(kind, HydraKind::HydraCms | HydraKind::HydraKll)

‎crates/executor/src/summary_kernels/datasketches_kll.rs‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,11 +54,13 @@ impl AggregateCore for DatasketchesKLLAccumulator {
5454
.as_any()
5555
.downcast_ref::<Self>()
5656
.ok_or("KLL merges only with KLL")?;
57-
if self.inner.k() != other.inner.k() {
57+
// sketchlib's merge panics on a different k or m.
58+
let (k, m) = (self.inner.k(), self.inner.m());
59+
if (k, m) != (other.inner.k(), other.inner.m()) {
5860
return Err(format!(
59-
"KLL merges only with the same k ({} vs {})",
60-
self.inner.k(),
61-
other.inner.k()
61+
"KLL merges only with the same k and m ({k}, {m} vs {}, {})",
62+
other.inner.k(),
63+
other.inner.m()
6264
)
6365
.into());
6466
}
@@ -136,13 +138,18 @@ mod tests {
136138
assert!(kll.approx_memory_bytes() > empty);
137139
}
138140

139-
// States of different k do not merge.
141+
// States of different k or m do not merge: an error, not sketchlib's panic.
140142
#[test]
141-
fn merge_rejects_a_different_k() {
143+
fn merge_rejects_a_different_k_or_m() {
142144
let kll = DatasketchesKLLAccumulator::new(200);
143145
assert!(kll
144146
.merge_with(&DatasketchesKLLAccumulator::new(100))
145147
.is_err());
148+
assert!(kll
149+
.merge_with(&DatasketchesKLLAccumulator {
150+
inner: Kll::new(200, 4),
151+
})
152+
.is_err());
146153
assert!(kll
147154
.merge_with(&DatasketchesKLLAccumulator::new(200))
148155
.is_ok());

‎crates/executor/src/summary_kernels/dd_sketch.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,12 @@ impl AggregateCore for DDSketchAccumulator {
4242
self.inner.get_count() == 0
4343
}
4444

45+
/// sketchlib's count of what the sketch holds now (its struct
46+
/// included); the bucket stores grow with the values' range.
47+
fn approx_memory_bytes(&self) -> usize {
48+
self.inner.memory_bytes()
49+
}
50+
4551
/// Quantiles.
4652
fn estimate(&self, query: &SketchStatistic) -> Result<f64, KernelError> {
4753
match query {

‎crates/executor/src/summary_kernels/factory.rs‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -263,11 +263,7 @@ impl AccumulatorUpdater for DDSketchAccumulatorUpdater {
263263
}
264264

265265
fn memory_usage_bytes(&self) -> usize {
266-
// The struct and the 8-byte counts both bucket stores allocate,
267-
// which grow with the values' range.
268-
let sketch = &self.acc.inner;
269-
let bins = sketch.positive_store_counts().len() + sketch.negative_store_counts().len();
270-
std::mem::size_of::<DDSketchAccumulator>() + bins * std::mem::size_of::<u64>()
266+
self.acc.approx_memory_bytes()
271267
}
272268
}
273269

@@ -716,10 +712,8 @@ mod planner_parameter_regression {
716712
},
717713
),
718714
] {
719-
let family = SummaryFamilyType::Sketch(
720-
SketchKind::new(algorithm.clone(), params),
721-
Default::default(),
722-
);
715+
let family =
716+
SummaryFamilyType::Sketch(SketchKind::new(algorithm, params), Default::default());
723717
let update = SummaryUpdate {
724718
item: Some(SummaryInputExpr::Column(
725719
planner_types::ir::scalar::ColumnRef::Named("host".into()),
@@ -872,9 +866,9 @@ mod ddsketch_domain {
872866
mod tests {
873867
use super::*;
874868

875-
/// A DDSketch updater reports the bins its stores allocate: none
876-
/// before a sample, one 128-bin chunk of the positive store after a
877-
/// few positive samples.
869+
/// A DDSketch updater reports sketchlib's count of the bins its stores
870+
/// allocate: none before a sample, one 128-bin chunk of the positive
871+
/// store after a few positive samples.
878872
#[test]
879873
fn ddsketch_memory_is_its_allocated_bins() {
880874
let mut updater = DDSketchAccumulatorUpdater::new(0.05);
@@ -884,6 +878,10 @@ mod tests {
884878
updater.update_single(value, 0).unwrap();
885879
}
886880
assert_eq!(updater.memory_usage_bytes(), empty + 128 * 8);
881+
assert_eq!(
882+
updater.memory_usage_bytes(),
883+
updater.acc.inner.memory_bytes()
884+
);
887885
}
888886

889887
/// A numeric sketch takes an Int64 as its exact Float64, and refuses

‎crates/executor/src/summary_kernels/univmon.rs‎

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -160,29 +160,30 @@ impl UnivMonAccumulator {
160160
}
161161

162162
impl AggregateCore for UnivMonAccumulator {
163-
/// Counters and heap slots of every layer, plus an upper bound on the
164-
/// variable-length keys the heaps retain: the smaller of every slot
165-
/// holding the longest key seen and every layer holding a copy of every
166-
/// key inserted. Constant time, since a build charges memory after
167-
/// every row; the library exposes no eviction hook for an exact running
168-
/// total, so a stream of long keys is still overcounted.
163+
/// This accumulator's own fields, sketchlib's bytes for the sketch's
164+
/// shape with keys that own no bytes ([`UnivMon::estimated_memory_bytes`]:
165+
/// at least what it holds), and an upper bound on the variable-length
166+
/// keys the heaps retain: the smaller of every slot holding the longest
167+
/// key seen and every layer holding a copy of every key inserted.
168+
/// Constant time, since a build charges memory after every row and
169+
/// [`UnivMon::memory_bytes`] walks the heaps; the library exposes no
170+
/// eviction hook for an exact running total, so a stream of long keys is
171+
/// still overcounted.
169172
fn approx_memory_bytes(&self) -> usize {
170-
let layers = self.inner.layers;
173+
let (heap_size, rows, cols, layers) = self.dimensions();
171174
let key_bytes = layers
172-
.saturating_mul(self.inner.heap_size)
175+
.saturating_mul(heap_size)
173176
.saturating_mul(self.max_key_bytes)
174177
.min(layers.saturating_mul(self.inserted_key_bytes));
175-
key_bytes
176-
.saturating_add(std::mem::size_of::<Self>())
177-
.saturating_add(
178-
self.inner.layers.saturating_mul(
179-
self.inner
180-
.rows
181-
.saturating_mul(self.inner.cols)
182-
.saturating_mul(16)
183-
.saturating_add(self.inner.heap_size.saturating_mul(256)),
184-
),
185-
)
178+
(std::mem::size_of::<Self>() - std::mem::size_of::<UnivMon>())
179+
.saturating_add(UnivMon::estimated_memory_bytes(
180+
heap_size,
181+
rows,
182+
cols,
183+
layers,
184+
&DataInput::U64(0),
185+
))
186+
.saturating_add(key_bytes)
186187
}
187188
fn clone_boxed_core(&self) -> Box<dyn AggregateCore> {
188189
Box::new(self.clone())

‎crates/executor/src/supported_summaries.rs‎

Lines changed: 21 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,24 +3,18 @@
33
//! `DeploymentCapabilities::supported_summaries`, so the Planner offers no
44
//! summary the executor would reject when it binds the plan.
55
6-
use crate::capability::{EVALUATED_FAMILIES, UNEVALUATED_READOUTS};
7-
use planner_types::ir::schema::{FamilyCapability, SupportedSummary};
6+
use crate::capability::EVALUATED_READOUTS;
7+
use planner_types::ir::schema::SupportedSummary;
88

99
/// Every sketch family this executor builds from rows and the statistics it
10-
/// answers from one state of it: the family-capability table's row of each
11-
/// family it evaluates, less the readouts it does not evaluate yet. Exact
12-
/// accumulators are always supported and are not listed.
10+
/// answers from one state of it (`EVALUATED_READOUTS`). Exact accumulators
11+
/// are always supported and are not listed.
1312
pub fn supported_summary_families() -> Vec<SupportedSummary> {
14-
EVALUATED_FAMILIES
15-
.into_iter()
16-
.map(|family| SupportedSummary {
17-
readouts: FamilyCapability::of_per_instance_sketch(&family)
18-
.readouts
19-
.iter()
20-
.map(|&(readout, _)| readout)
21-
.filter(|readout| !UNEVALUATED_READOUTS.contains(&(family.clone(), *readout)))
22-
.collect(),
13+
EVALUATED_READOUTS
14+
.iter()
15+
.map(|&(family, readouts)| SupportedSummary {
2316
family,
17+
readouts: readouts.to_vec(),
2418
})
2519
.collect()
2620
}
@@ -34,20 +28,20 @@ mod tests {
3428
use planner_types::ir::operator::operator_properties::{Reduction, Source};
3529
use planner_types::ir::scalar::ColumnRef;
3630
use planner_types::ir::schema::{
37-
DataType, Field, FieldDataType, GroupingStrategy, Schema, SketchAlgorithm, SketchKind,
38-
SketchParams, SketchStatistic, StatisticKind, SummaryInputExpr, SummaryUpdate,
31+
DataType, FamilyCapability, Field, FieldDataType, GroupingStrategy, Schema,
32+
SketchAlgorithm, SketchKind, SketchParams, SketchStatistic, StatisticKind,
33+
SummaryInputExpr, SummaryUpdate,
3934
};
4035
use planner_types::ir::{ASAPOp, NonASAPOp, Operator, OperatorNode};
4136

42-
const ALGORITHMS: [SketchAlgorithm; 10] = [
37+
const ALGORITHMS: [SketchAlgorithm; 9] = [
4338
SketchAlgorithm::UnivMon,
4439
SketchAlgorithm::Kll,
4540
SketchAlgorithm::Cms,
4641
SketchAlgorithm::Hll,
4742
SketchAlgorithm::DDSketch,
4843
SketchAlgorithm::CmsWithHeap,
4944
SketchAlgorithm::Kmv,
50-
SketchAlgorithm::Theta,
5145
SketchAlgorithm::CountSketch,
5246
SketchAlgorithm::CountSketchWithHeap,
5347
];
@@ -95,7 +89,6 @@ mod tests {
9589
k: 1024,
9690
relative_error: None,
9791
},
98-
A::Theta => SketchParams::Theta { k: 1024 },
9992
A::CountSketch => SketchParams::CountSketch {
10093
width: 64,
10194
depth: 3,
@@ -105,12 +98,13 @@ mod tests {
10598
depth: 3,
10699
heap_size: 10,
107100
},
101+
other => unreachable!("{other:?} is not probed"),
108102
}
109103
}
110104

111105
fn per_instance(algorithm: &SketchAlgorithm) -> FieldDataType {
112106
FieldDataType::Sketch(
113-
SketchKind::new(algorithm.clone(), params(algorithm)),
107+
SketchKind::new(*algorithm, params(algorithm)),
114108
GroupingStrategy::PerSubpopulationInstance,
115109
)
116110
}
@@ -133,6 +127,7 @@ mod tests {
133127
StatisticKind::FrequencyL2 => SketchStatistic::FrequencyL2,
134128
StatisticKind::FrequencyEntropy => SketchStatistic::FrequencyEntropy,
135129
StatisticKind::TopK => SketchStatistic::TopK { k: 5 },
130+
other => unreachable!("{other:?} is not probed"),
136131
}
137132
}
138133

@@ -230,9 +225,8 @@ mod tests {
230225
/// The list holds exactly the (family, readout) pairs the factory builds
231226
/// per instance and an evaluation answers from that state, and the
232227
/// IR's `SummaryEstimate` validation accepts exactly what the
233-
/// family-capability table answers. A listed family's readouts are its
234-
/// table row less the ones named not evaluated, each of which the table
235-
/// answers and the executor does not.
228+
/// family-capability table answers. Every declared readout is in the
229+
/// family's table row.
236230
#[test]
237231
fn supported_summary_families_match_the_factory_and_evaluations() {
238232
let listed = supported_summary_families();
@@ -252,11 +246,10 @@ mod tests {
252246
answered,
253247
"IR: {algorithm:?} {readout:?}"
254248
);
255-
if UNEVALUATED_READOUTS.contains(&(algorithm.clone(), readout)) {
256-
assert!(answered && !executed, "{algorithm:?} {readout:?}");
257-
} else if EVALUATED_FAMILIES.contains(algorithm) {
258-
assert_eq!(answered, executed, "table: {algorithm:?} {readout:?}");
259-
}
249+
assert!(
250+
!declared || answered,
251+
"declared but not in the table: {algorithm:?} {readout:?}"
252+
);
260253
}
261254
}
262255
assert!(listed.iter().all(|s| !s.readouts.is_empty()));

‎crates/executor/tests/deployment.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ fn factory_and_dag_heap_states_are_interchangeable() {
311311
FrequencyAlgorithm::CountSketch,
312312
),
313313
] {
314-
let family = heap_sketch(algorithm.clone(), 1_024, 5, 4);
314+
let family = heap_sketch(algorithm, 1_024, 5, 4);
315315
let factory = build_weighted_frequency(&family, [(0, 1.0), (1, 2.0)]);
316316
let mut dag = WeightedFrequency::new(frequency, 1_024, 5, 4).unwrap();
317317
dag.update(&[Value::Utf8("host-0".into())], 3.0).unwrap();

0 commit comments

Comments
 (0)