Skip to content

Commit c0bb156

Browse files
committed
refactor(plan-selection)!: drop AccuracyModel::propagate; name Hydra Count-Min in the weights rejection
Stage 3 composed through the model only for a ratio of relative estimates with enforced operand domains (domain_bound, ExactDivision); every other guarantee comes from Stage 1's built-in rules. The trait now has satisfies and accepts_output_regardless_of_guarantee, and domain_bound calls accuracy::composition::propagate directly. A Hydra (SharedMultiSubpopulation) summary is now named by its Hydra kind, so the unproven-weights reason reads "HydraCms needs non-negative update weights" rather than "Cms". Document which Count-Min forms the weight check covers and why: the executor refuses CmsWithHeap and HydraCms at plan build, and plain Count-Min fails at the first negative weight. BREAKING CHANGE: AccuracyModel::propagate is removed; a deployment model cannot change how guarantees compose.
1 parent 775b04b commit c0bb156

3 files changed

Lines changed: 94 additions & 45 deletions

File tree

‎crates/plan-selection/src/accuracy.rs‎

Lines changed: 19 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -4,42 +4,26 @@
44
//! Stage 1 composes each candidate's guarantee with the planner's built-in
55
//! analytical rules (`asap_logical_asap_optimization::accuracy`) and keeps a
66
//! candidate it has no rule for with `guarantee = None`. This stage decides,
7-
//! with the model here, whether a guarantee meets its query's target, how
8-
//! guarantees compose once the data workload and the deployment's evidence
9-
//! ([`AccuracyEvidenceProvider`]) bind the statistics Stage 1 left unknown,
7+
//! with the model here, whether a guarantee meets its query's target (after
8+
//! the data workload and the deployment's evidence,
9+
//! [`AccuracyEvidenceProvider`], bind the statistics Stage 1 left unknown),
1010
//! and whether the deployment accepts an output directly, whatever its
1111
//! guarantee says.
1212
13-
use asap_logical_asap_optimization::accuracy::{
14-
self as analytical, OperandDomain, PropagationStats,
15-
};
16-
use asap_types::ir::properties::{
17-
AccuracyError, AccuracyGuarantee, AccuracyTarget, CompositionOperator,
18-
};
13+
use asap_logical_asap_optimization::accuracy::{self as analytical, OperandDomain};
14+
use asap_types::ir::properties::{AccuracyGuarantee, AccuracyTarget};
1915
use asap_types::ir::{OperatorNode, QueryRoot};
2016

21-
/// The deployment-extensible accuracy algebra plan selection uses. This
22-
/// crate ships [`BuiltinAccuracyModel`]; a deployment with a proof for a
23-
/// composition the default rejects (a registered cross-metric conversion,
24-
/// say) implements this trait and passes it in
17+
/// The deployment's accuracy decisions in plan selection: whether a
18+
/// guarantee meets a query's target, and whether the deployment accepts an
19+
/// output directly, whatever its guarantee. This crate ships
20+
/// [`BuiltinAccuracyModel`]; a deployment with its own target check (a
21+
/// registered cross-metric conversion, say) or its own measurements
22+
/// implements this trait and passes it in
2523
/// [`PlanningModels::accuracy`](crate::PlanningModels::accuracy).
24+
/// Guarantees are composed with the built-in rules only
25+
/// (`asap_logical_asap_optimization::accuracy::composition::propagate`).
2626
pub trait AccuracyModel {
27-
/// Compose `inputs`' guarantees (in the parent's child order) with the
28-
/// parent's own `local` guarantee under `op`, given the statistics
29-
/// plan selection bound (`stats`). `Err` is the fail-closed answer: no
30-
/// registered rule, or a missing input guarantee. The default is the
31-
/// built-in rules (`asap_logical_asap_optimization::accuracy::composition::propagate`);
32-
/// a deployment overrides it only to add a rule.
33-
fn propagate(
34-
&self,
35-
op: &CompositionOperator,
36-
inputs: &[AccuracyGuarantee],
37-
local: Option<&AccuracyGuarantee>,
38-
stats: &PropagationStats,
39-
) -> Result<AccuracyGuarantee, AccuracyError> {
40-
analytical::composition::propagate(op, inputs, local, stats)
41-
}
42-
4327
/// Whether `guarantee` meets `target`: a bound within its ε, and within
4428
/// its δ when it states one, over the values the target's scope covers
4529
/// (a per-value δ does not meet a whole-result target). When the target
@@ -56,9 +40,11 @@ pub trait AccuracyModel {
5640
/// this for every query output before any guarantee check; when it
5741
/// returns `true`, no guarantee or evidence check runs for that output.
5842
/// It does not override the executability check before it: a plan that
59-
/// reads Count-Min over update weights not proven non-negative is
60-
/// rejected (`RejectionCause::CountMinWeightsNotProvenNonNegative`)
61-
/// whatever this returns. The default accepts
43+
/// reads Count-Min (plain, CmsWithHeap or Hydra Count-Min) over update
44+
/// weights not proven non-negative is rejected
45+
/// (`RejectionCause::CountMinWeightsNotProvenNonNegative`) whatever this
46+
/// returns, because the executor refuses CmsWithHeap and HydraCms when it
47+
/// builds the plan and plain Count-Min fails at the first negative weight. The default accepts
6248
/// none, so Stage 3 checks the Stage 1 guarantee, then the evidence
6349
/// ([`AccuracyEvidenceProvider::output_guarantee`]) with
6450
/// [`Self::satisfies`].
@@ -72,7 +58,7 @@ pub trait AccuracyModel {
7258
}
7359

7460
/// The planner's built-in analytical rules as an [`AccuracyModel`]: the
75-
/// trait's default composition, the conservative target check Stage 1's
61+
/// conservative target check Stage 1's
7662
/// `accuracy::satisfies` makes, and no output accepted regardless of its
7763
/// guarantee.
7864
/// The default in [`PlanningModels::builtin`](crate::PlanningModels::builtin).

‎crates/plan-selection/src/lib.rs‎

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1475,9 +1475,12 @@ fn accuracy_check(
14751475
}
14761476
_ => String::new(),
14771477
};
1478-
// The executor cannot run Count-Min over weights not proven
1479-
// non-negative, so this rejects the plan whatever the
1480-
// deployment's accuracy model accepts.
1478+
// Count-Min (plain, CmsWithHeap or Hydra Count-Min) over
1479+
// weights not proven non-negative does not run: the executor
1480+
// refuses CmsWithHeap and HydraCms when it builds the plan,
1481+
// and plain Count-Min fails at the first negative weight. So
1482+
// this rejects the plan whatever the deployment's accuracy
1483+
// model accepts.
14811484
return Err((
14821485
RejectionCause::CountMinWeightsNotProvenNonNegative,
14831486
format!(
@@ -1895,13 +1898,16 @@ fn domain_bound(
18951898
division_operand_domains: Some([dx, dy]),
18961899
..PropagationStats::default()
18971900
};
1898-
return models
1899-
.accuracy
1900-
.propagate(&CompositionOperator::ExactDivision, &[gx, gy], None, &stats)
1901-
.ok()?
1902-
.spec
1903-
.error
1904-
.evaluate();
1901+
return asap_logical_asap_optimization::accuracy::composition::propagate(
1902+
&CompositionOperator::ExactDivision,
1903+
&[gx, gy],
1904+
None,
1905+
&stats,
1906+
)
1907+
.ok()?
1908+
.spec
1909+
.error
1910+
.evaluate();
19051911
}
19061912
let valid =
19071913
|d: &OperandDomain| d.lower.is_finite() && d.upper.is_finite() && d.lower <= d.upper;
@@ -1940,6 +1946,10 @@ fn domain_bound(
19401946

19411947
fn family_name(family: &FieldDataType) -> String {
19421948
match family {
1949+
// A shared (Hydra) structure is named by its Hydra kind, e.g. `HydraCms`.
1950+
FieldDataType::Sketch(_, GroupingStrategy::SharedMultiSubpopulation { kind, .. }) => {
1951+
format!("{kind:?}")
1952+
}
19431953
FieldDataType::Sketch(kind, _) => format!("{:?}", kind.algorithm()),
19441954
FieldDataType::ExactAggregate(kind, _) => format!("exact {kind:?} accumulator"),
19451955
other => format!("{other:?}"),
@@ -4337,6 +4347,60 @@ mod tests {
43374347
}
43384348
}
43394349

4350+
/// `node` with every CmsWithHeap summary build replaced by a Hydra
4351+
/// Count-Min over the same update.
4352+
fn with_hydra_count_min(node: &Rc<OperatorNode>) -> Rc<OperatorNode> {
4353+
use asap_types::ir::schema::{HydraKind, HydraParams, SketchParams};
4354+
let mut rebuilt = node.with_new_children(with_hydra_count_min).unwrap();
4355+
if let Operator::ASAP(ASAPOp::SummaryAgg { family, .. }) = &mut rebuilt.operator {
4356+
if let FieldDataType::Sketch(kind, _) = family {
4357+
if let SketchParams::CmsWithHeap { width, depth, .. } = kind.params() {
4358+
let (width, depth) = (*width, *depth);
4359+
*family = FieldDataType::Sketch(
4360+
SketchKind::new(SketchAlgorithm::Cms, SketchParams::Cms { width, depth }),
4361+
GroupingStrategy::SharedMultiSubpopulation {
4362+
kind: HydraKind::HydraCms,
4363+
params: HydraParams::HydraCms {
4364+
width,
4365+
depth,
4366+
shared_rows: depth,
4367+
shared_columns: width,
4368+
},
4369+
},
4370+
);
4371+
}
4372+
}
4373+
}
4374+
Rc::new(rebuilt)
4375+
}
4376+
4377+
/// A Hydra Count-Min over unproven weights is rejected by its own name:
4378+
/// the reason says `HydraCms`, not `Cms`.
4379+
#[test]
4380+
fn unproven_hydra_count_min_weights_name_hydra_cms() {
4381+
let target = AccuracyTarget::max_error(0.01)
4382+
.failure_probability(0.001)
4383+
.accuracy_metric(AccuracyMetric::ABSOLUTE_STREAM_L1);
4384+
let gauge = with_metric_types(data(), &[("m", MetricType::Gauge)]);
4385+
let mut p2 = candidates().into_iter().find(|c| c.id == "P2").unwrap();
4386+
p2.roots = p2
4387+
.roots
4388+
.iter()
4389+
.map(|root| match root {
4390+
QueryRoot::Operator(node) => QueryRoot::Operator(with_hydra_count_min(node)),
4391+
other => other.clone(),
4392+
})
4393+
.collect();
4394+
let (cause, reason) =
4395+
accuracy_check(&p2, &[target], &gauge, &PlanningModels::builtin()).unwrap_err();
4396+
assert_eq!(cause, RejectionCause::CountMinWeightsNotProvenNonNegative);
4397+
assert!(
4398+
reason.contains("HydraCms needs non-negative update weights"),
4399+
"{reason}"
4400+
);
4401+
assert!(reason.contains("not declared a counter"), "{reason}");
4402+
}
4403+
43404404
/// The model's hook has priority over every guarantee. The summary
43414405
/// candidates of a difference of quantile estimates have no theoretical
43424406
/// guarantee: with evidence that misses the target the built-in model

‎crates/planner/tests/deployment_accuracy_model.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ use asap_types::workload::{
2020
};
2121

2222
/// A deployment that trusts no approximate estimate: only an exact
23-
/// guarantee meets a target. Composition is the trait's default (the
24-
/// built-in rules).
23+
/// guarantee meets a target.
2524
struct ExactOnlyAccuracyModel;
2625

2726
impl AccuracyModel for ExactOnlyAccuracyModel {

0 commit comments

Comments
 (0)