Skip to content

Commit fedc920

Browse files
committed
feat(plan-selection)!: let the deployment's accuracy model accept an output regardless of its guarantee
The hook now has priority over every guarantee: Stage 3 asks AccuracyModel::accepts_output_regardless_of_guarantee first for each query output, and an output it accepts skips the guarantee, Count-Min weight and evidence checks. Otherwise the check is unchanged (Stage 1 guarantee, then evidence, then AccuracyTargetMissed, or NoAccuracyGuarantee when neither gives one). The default returns false, so the built-in models behave as before. BREAKING CHANGE: AccuracyModel::accepts_output_without_guarantee is now accepts_output_regardless_of_guarantee, and AcceptedAccuracy::accepted_without_guarantee (stage_pipeline JSON key accepted_without_guarantee) is now accepted_regardless_of_guarantee.
1 parent 1910429 commit fedc920

5 files changed

Lines changed: 225 additions & 202 deletions

File tree

‎crates/devtools/src/bin/stage_pipeline.rs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,10 @@
3131
// - stage3_plan_selection: the selected candidate, chosen among all candidates;
3232
// the shown candidates' costs and accepted accuracy (per query: the
3333
// target's metric or null when not set, the accepted guarantee's
34-
// metric or null when exact or accepted without a guarantee, whether
35-
// the deployment's accuracy model accepted it without a guarantee
36-
// (`accepted_without_guarantee`), the sketches read and a one-line
34+
// metric or null when exact or accepted regardless of guarantee,
35+
// whether the deployment's accuracy model accepted it directly,
36+
// whatever its guarantee says (`accepted_regardless_of_guarantee`),
37+
// the sketches read and a one-line
3738
// summary), and every other shown candidate under `rejected` with
3839
// its `cause`: a failed check (`failure_probability_resize_failed`,
3940
// `accuracy_target_missed`, `no_accuracy_guarantee`, `raw_data_not_retained`,
@@ -619,7 +620,7 @@ fn plan_selection_json(
619620
json!({
620621
"target_metric": accepted.target_metric,
621622
"accepted_metric": accepted.accepted_metric,
622-
"accepted_without_guarantee": accepted.accepted_without_guarantee,
623+
"accepted_regardless_of_guarantee": accepted.accepted_regardless_of_guarantee,
623624
"sketches": accepted.sketches,
624625
"summary": accepted.to_string(),
625626
})

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

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
//! with the model here, whether a guarantee meets its query's target, how
88
//! guarantees compose once the data workload and the deployment's evidence
99
//! ([`AccuracyEvidenceProvider`]) bind the statistics Stage 1 left unknown,
10-
//! and whether an output with no guarantee at all is accepted.
10+
//! and whether the deployment accepts an output directly, whatever its
11+
//! guarantee says.
1112
1213
use asap_logical_asap_optimization::accuracy::{
1314
self as analytical, OperandDomain, PropagationStats,
@@ -50,13 +51,14 @@ pub trait AccuracyModel {
5051
fn satisfies(&self, guarantee: &AccuracyGuarantee, target: &AccuracyTarget) -> bool;
5152

5253
/// Whether the deployment accepts `output`, one query's output in a
53-
/// candidate, for `target` although the output has no guarantee: Stage 1
54-
/// had no analytical rule for it and the deployment's evidence supplies
55-
/// none ([`AccuracyEvidenceProvider::output_guarantee`]). Stage 3 asks only
56-
/// then; a guarantee from either source is checked with
57-
/// [`Self::satisfies`] instead. The default accepts none, and Stage 3
58-
/// rejects such a candidate (`RejectionCause::NoAccuracyGuarantee`).
59-
fn accepts_output_without_guarantee(
54+
/// candidate, for `target` directly, whatever its guarantee says:
55+
/// missing (Stage 1 had no analytical rule), met or missed. Stage 3 asks
56+
/// this first for every query output; when it returns `true`, no
57+
/// guarantee or evidence check runs for that output. The default accepts
58+
/// none, so Stage 3 checks the Stage 1 guarantee, then the evidence
59+
/// ([`AccuracyEvidenceProvider::output_guarantee`]) with
60+
/// [`Self::satisfies`].
61+
fn accepts_output_regardless_of_guarantee(
6062
&self,
6163
_output: &QueryRoot,
6264
_target: &AccuracyTarget,
@@ -67,7 +69,8 @@ pub trait AccuracyModel {
6769

6870
/// The planner's built-in analytical rules as an [`AccuracyModel`]: the
6971
/// trait's default composition, the conservative target check Stage 1's
70-
/// `accuracy::satisfies` makes, and no output accepted without a guarantee.
72+
/// `accuracy::satisfies` makes, and no output accepted regardless of its
73+
/// guarantee.
7174
/// The default in [`PlanningModels::builtin`](crate::PlanningModels::builtin).
7275
#[derive(Debug, Default, Clone, Copy)]
7376
pub struct BuiltinAccuracyModel;

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

Lines changed: 80 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@
77
//! from the data workload and the deployment's operand-domain contracts),
88
//! or when the deployment's empirical evidence shows this
99
//! candidate's output does (`AccuracyEvidenceProvider::output_guarantee`,
10-
//! read only here). An output with no guarantee from either source is
11-
//! accepted only when the deployment's accuracy model accepts it without one
12-
//! (`AccuracyModel::accepts_output_without_guarantee`). For a query with a target, Count-Min is admitted only
10+
//! read only here). Before either, the deployment's accuracy model may accept
11+
//! the output directly, whatever its guarantee says
12+
//! (`AccuracyModel::accepts_output_regardless_of_guarantee`). For a query with a target, Count-Min is admitted only
1313
//! over weights proven non-negative; a query without one constrains nothing.
1414
//! A miss rejects the candidate as invalid, with a reason; only this stage
1515
//! discards valid candidates.
@@ -589,8 +589,8 @@ pub enum RejectionCause {
589589
AccuracyTargetMissed,
590590
/// A query's output has no accuracy guarantee (Stage 1 has no
591591
/// analytical rule for it and the deployment's evidence supplies none),
592-
/// and the deployment's accuracy model does not accept it without one
593-
/// ([`AccuracyModel::accepts_output_without_guarantee`]).
592+
/// and the deployment's accuracy model did not accept it directly
593+
/// ([`AccuracyModel::accepts_output_regardless_of_guarantee`]).
594594
NoAccuracyGuarantee,
595595
/// It scans raw data at query time in a deployment that does not
596596
/// retain raw data.
@@ -645,14 +645,14 @@ pub struct AcceptedAccuracy {
645645
/// The target's metric; `None` when the user did not set one.
646646
pub target_metric: Option<AccuracyMetric>,
647647
/// The metric the accepted guarantee is stated in; `None` for an exact
648-
/// output, which meets every metric, and for one accepted without a
649-
/// guarantee.
648+
/// output, which meets every metric, and for one the deployment's model
649+
/// accepted regardless of its guarantee (no guarantee was checked).
650650
pub accepted_metric: Option<AccuracyMetric>,
651-
/// Whether the output has no guarantee and the deployment's accuracy
652-
/// model accepted it anyway
653-
/// ([`AccuracyModel::accepts_output_without_guarantee`]); `false`
654-
/// otherwise.
655-
pub accepted_without_guarantee: bool,
651+
/// Whether the deployment's accuracy model accepted the output directly,
652+
/// whatever its guarantee says
653+
/// ([`AccuracyModel::accepts_output_regardless_of_guarantee`]); `false`
654+
/// when a guarantee met the target.
655+
pub accepted_regardless_of_guarantee: bool,
656656
/// The sketch algorithms the query's output reads, in plan order
657657
/// (e.g. `["Kll"]`); empty for an exact plan.
658658
pub sketches: Vec<String>,
@@ -661,7 +661,7 @@ pub struct AcceptedAccuracy {
661661
impl std::fmt::Display for AcceptedAccuracy {
662662
/// E.g. `metric not set; accepted rank (Kll)`, or
663663
/// `metric relative; accepted relative (DDSketch)`, or
664-
/// `metric rank; accepted without a guarantee (Kll)`.
664+
/// `metric rank; accepted by the deployment's accuracy model regardless of guarantee (Kll)`.
665665
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
666666
let name = |metric: AccuracyMetric| {
667667
serde_json::to_value(metric)
@@ -674,7 +674,10 @@ impl std::fmt::Display for AcceptedAccuracy {
674674
Some(metric) => write!(f, "metric {}; ", name(metric))?,
675675
}
676676
match self.accepted_metric {
677-
_ if self.accepted_without_guarantee => write!(f, "accepted without a guarantee")?,
677+
_ if self.accepted_regardless_of_guarantee => write!(
678+
f,
679+
"accepted by the deployment's accuracy model regardless of guarantee"
680+
)?,
678681
None => write!(f, "accepted exact")?,
679682
Some(metric) => write!(f, "accepted {}", name(metric))?,
680683
}
@@ -1416,12 +1419,13 @@ pub fn resize_for_failure_probability(
14161419
/// its ε, δ and scope ([`AccuracyModel::satisfies`]); one that sets a metric
14171420
/// accepts only guarantees in it (or convertible to it).
14181421
///
1419-
/// An output with no guarantee from either source (Stage 1 had no rule,
1420-
/// and the evidence has none) is accepted only when the deployment's
1421-
/// accuracy model accepts it without one
1422-
/// ([`AccuracyModel::accepts_output_without_guarantee`]); otherwise the
1423-
/// candidate is rejected as [`RejectionCause::NoAccuracyGuarantee`]. Any
1424-
/// other miss is [`RejectionCause::AccuracyTargetMissed`].
1422+
/// The deployment's accuracy model is asked first
1423+
/// ([`AccuracyModel::accepts_output_regardless_of_guarantee`]): an output it
1424+
/// accepts is accepted whatever its guarantee says, and none of the checks
1425+
/// above runs for it. Otherwise, an output with no guarantee from either
1426+
/// source (Stage 1 had no rule, and the evidence has none) is rejected as
1427+
/// [`RejectionCause::NoAccuracyGuarantee`], and any other miss as
1428+
/// [`RejectionCause::AccuracyTargetMissed`].
14251429
fn accuracy_check(
14261430
candidate: &PhysicalCandidate,
14271431
targets: &[AccuracyTarget],
@@ -1432,6 +1436,9 @@ fn accuracy_check(
14321436
for (query, (root, target)) in candidate.roots.iter().zip(targets).enumerate() {
14331437
let query = query + 1;
14341438
let mut sketches: Vec<String> = Vec::new();
1439+
// The first Count-Min whose update weights are not proven
1440+
// non-negative, which its bound assumes, as a rejection reason.
1441+
let mut unproven_weights: Option<String> = None;
14351442
for node in root_operators(root)
14361443
.iter()
14371444
.flat_map(|node| OperatorNode::reachable(node))
@@ -1461,16 +1468,32 @@ fn accuracy_check(
14611468
}
14621469
_ => String::new(),
14631470
};
1464-
return Err((
1465-
RejectionCause::AccuracyTargetMissed,
1471+
unproven_weights.get_or_insert_with(|| {
14661472
format!(
14671473
"q{query}: {} needs non-negative update weights, and these are not \
14681474
proven non-negative{unconfirmed}",
14691475
family_name(family)
1470-
),
1471-
));
1476+
)
1477+
});
14721478
}
14731479
}
1480+
// The deployment's model may accept the output directly, whatever
1481+
// its guarantee says; no guarantee or evidence check runs then.
1482+
if models
1483+
.accuracy
1484+
.accepts_output_regardless_of_guarantee(root, target)
1485+
{
1486+
accepted.push(AcceptedAccuracy {
1487+
target_metric: target.accuracy_metric,
1488+
accepted_metric: None,
1489+
accepted_regardless_of_guarantee: true,
1490+
sketches,
1491+
});
1492+
continue;
1493+
}
1494+
if let Some(reason) = unproven_weights {
1495+
return Err((RejectionCause::AccuracyTargetMissed, reason));
1496+
}
14741497
// A guarantee per value is stated over the whole result's `G`
14751498
// values for a whole-result target.
14761499
let series = series_read(root, data);
@@ -1495,31 +1518,17 @@ fn accuracy_check(
14951518
accepted.push(AcceptedAccuracy {
14961519
target_metric: target.accuracy_metric,
14971520
accepted_metric: (!met.is_exact()).then_some(met.spec.accuracy_metric),
1498-
accepted_without_guarantee: false,
1521+
accepted_regardless_of_guarantee: false,
14991522
sketches,
15001523
});
15011524
continue;
15021525
}
1503-
// No guarantee from either source: the deployment's model decides.
15041526
if guarantee.is_none() && evidence.is_none() {
1505-
if models
1506-
.accuracy
1507-
.accepts_output_without_guarantee(root, target)
1508-
{
1509-
accepted.push(AcceptedAccuracy {
1510-
target_metric: target.accuracy_metric,
1511-
accepted_metric: None,
1512-
accepted_without_guarantee: true,
1513-
sketches,
1514-
});
1515-
continue;
1516-
}
15171527
return Err((
15181528
RejectionCause::NoAccuracyGuarantee,
15191529
format!(
1520-
"q{query}: the output has no guarantee, the deployment has no evidence \
1521-
for it, and its accuracy model does not accept it for target {target:?} \
1522-
without one"
1530+
"q{query}: the output has no guarantee, and the deployment has no evidence \
1531+
for it, so it cannot meet target {target:?}"
15231532
),
15241533
));
15251534
}
@@ -4262,15 +4271,15 @@ mod tests {
42624271
}
42634272
}
42644273

4265-
/// A model that accepts every output that has no guarantee.
4266-
struct AcceptsEveryOutputWithoutGuarantee;
4274+
/// A model that accepts every output directly, whatever its guarantee.
4275+
struct AcceptsEveryOutput;
42674276

4268-
impl AccuracyModel for AcceptsEveryOutputWithoutGuarantee {
4277+
impl AccuracyModel for AcceptsEveryOutput {
42694278
fn satisfies(&self, guarantee: &AccuracyGuarantee, target: &AccuracyTarget) -> bool {
42704279
BuiltinAccuracyModel.satisfies(guarantee, target)
42714280
}
42724281

4273-
fn accepts_output_without_guarantee(
4282+
fn accepts_output_regardless_of_guarantee(
42744283
&self,
42754284
_output: &QueryRoot,
42764285
_target: &AccuracyTarget,
@@ -4279,12 +4288,14 @@ mod tests {
42794288
}
42804289
}
42814290

4282-
/// The model's hook decides only when there is no guarantee at all: an
4283-
/// output with no theoretical guarantee whose evidence guarantee misses
4284-
/// the target is rejected as `AccuracyTargetMissed`, though the hook
4285-
/// would accept it. Without that evidence the hook accepts it.
4291+
/// The model's hook has priority over every guarantee. The summary
4292+
/// candidates of a difference of quantile estimates have no theoretical
4293+
/// guarantee: with evidence that misses the target the built-in model
4294+
/// rejects them as `AccuracyTargetMissed`, and with no evidence as
4295+
/// `NoAccuracyGuarantee`; a model that accepts every output accepts them
4296+
/// in both cases, regardless of guarantee.
42864297
#[test]
4287-
fn a_missed_evidence_guarantee_is_not_overridden_by_the_hook() {
4298+
fn the_accuracy_model_accepts_an_output_whatever_its_guarantee() {
42884299
let target = AccuracyTarget::max_error(0.01).failure_probability(0.001);
42894300
let candidates = all_candidates(
42904301
"quantile_over_time(0.99, m[1m]) - quantile_over_time(0.5, m[1m])",
@@ -4296,7 +4307,7 @@ mod tests {
42964307
.map(|c| c.id.as_str())
42974308
.collect();
42984309
assert!(!summaries.is_empty());
4299-
let select = |evidence: &dyn AccuracyEvidenceProvider| {
4310+
let select = |accuracy: &dyn AccuracyModel, evidence: &dyn AccuracyEvidenceProvider| {
43004311
select_plan(
43014312
&candidates,
43024313
&[entry_with(
@@ -4305,27 +4316,29 @@ mod tests {
43054316
)],
43064317
&data(),
43074318
PlanningModels::builtin()
4308-
.with_accuracy(&AcceptsEveryOutputWithoutGuarantee)
4319+
.with_accuracy(accuracy)
43094320
.with_evidence(evidence),
43104321
)
43114322
.unwrap()
43124323
};
4313-
let loose = select(&EveryOutputMeasuredLoose);
4314-
for id in &summaries {
4315-
let rejection = loose.rejected.iter().find(|r| r.id == *id).unwrap();
4316-
assert_eq!(
4317-
rejection.cause,
4324+
for (evidence, builtin_cause) in [
4325+
(
4326+
&EveryOutputMeasuredLoose as &dyn AccuracyEvidenceProvider,
43184327
RejectionCause::AccuracyTargetMissed,
4319-
"{}",
4320-
rejection.reason
4321-
);
4322-
}
4323-
let no_evidence = select(&NoAccuracyEvidence);
4324-
for id in &summaries {
4325-
assert!(
4326-
no_evidence.accepted[*id][0].accepted_without_guarantee,
4327-
"{id}"
4328-
);
4328+
),
4329+
(&NoAccuracyEvidence, RejectionCause::NoAccuracyGuarantee),
4330+
] {
4331+
let builtin = select(&BuiltinAccuracyModel, evidence);
4332+
let accepting = select(&AcceptsEveryOutput, evidence);
4333+
for id in &summaries {
4334+
let rejection = builtin.rejected.iter().find(|r| r.id == *id).unwrap();
4335+
assert_eq!(rejection.cause, builtin_cause, "{}", rejection.reason);
4336+
assert!(accepting.costs.contains_key(*id), "{id} is valid");
4337+
assert!(
4338+
accepting.accepted[*id][0].accepted_regardless_of_guarantee,
4339+
"{id}"
4340+
);
4341+
}
43294342
}
43304343
}
43314344

0 commit comments

Comments
 (0)