Skip to content

Commit fc328b1

Browse files
committed
refactor: name ASAP family strategies and clarify IR modules
1 parent c3a5af9 commit fc328b1

43 files changed

Lines changed: 305 additions & 282 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎crates/asap-aware-mapping/src/accuracy/mod.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use crate::exact_composition::ExactOperation;
3333
/// [`DefaultAccuracyModel`]; a deployment with a proof for a composition the
3434
/// default rejects (a registered cross-metric conversion, say) implements
3535
/// this trait and passes it to
36-
/// [`crate::replacement::SketchAlgorithmStrategy::new_with_planning_inputs`].
36+
/// [`crate::replacement::ASAPStrategies::new_with_planning_inputs`].
3737
pub trait AccuracyModel {
3838
/// The definition-registered rule for applying `operation` to an
3939
/// approximate input. `None` means the function is exact only over exact

‎crates/asap-aware-mapping/src/accuracy/reconciliation.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
//! Two `NonASAPOp::Aggregate` nodes are accuracy-near-duplicates here iff,
3232
//! **in this order**:
3333
//!
34-
//! 1. Both are the same bindable shape [`crate::replacement::SketchAlgorithmStrategy`]
34+
//! 1. Both are the same bindable shape [`crate::replacement::ASAPStrategies`]
3535
//! itself targets — a single measure, no `HAVING` (`bindable_intent`'s own
3636
//! scope) — **and** that one measure is one of the four accuracy-bearing
3737
//! [`AggIntent`] variants ([`crate::replacement::accuracy_target`]'s own
@@ -105,7 +105,7 @@
105105
//!
106106
//! Like every [`ReplacementStrategy`], this only ever *proposes* — the
107107
//! looser-accuracy consumer's own independently-sized candidate (from
108-
//! [`crate::replacement::SketchAlgorithmStrategy`]) stays in its
108+
//! [`crate::replacement::ASAPStrategies`]) stays in its
109109
//! [`crate::replacement::TargetSubDAGCandidates`] right alongside this strategy's
110110
//! "read the tighter sibling instead" [`Replacement::Rewrite`] candidate;
111111
//! [`crate::cost_model::CostModel`]-driven ranking picks between them;
@@ -176,7 +176,7 @@ type BindableAccuracyAggregate<'a> = (
176176

177177
/// The `(reduction, intent, accuracy, output_names, child)` shape this
178178
/// module operates on: the same single-measure, no-`HAVING` bindable shape
179-
/// [`crate::replacement::SketchAlgorithmStrategy`] targets (see that
179+
/// [`crate::replacement::ASAPStrategies`] targets (see that
180180
/// module's private `bindable_intent`), further narrowed to a measure whose
181181
/// intent actually carries an [`AccuracyTarget`]
182182
/// ([`crate::replacement::accuracy_target`]'s own scope: `Count` /
@@ -838,7 +838,7 @@ mod tests {
838838
"global_selection must commit to some candidate for a single-consumer looser target"
839839
);
840840
// With no recompute term at all (it never rebuilds `target`), this
841-
// candidate strictly undercuts every SketchAlgorithmStrategy
841+
// candidate strictly undercuts every ASAPStrategies
842842
// candidate (which each pay a recompute term on top of their own
843843
// maintenance term) under DefaultCostModel's numbers — the sane
844844
// direction: reading an already-necessary sibling should be able to

‎crates/asap-aware-mapping/src/cost_model.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
//! than overloading these ones across incompatible `Kind`/`Params` types.
2727
//!
2828
//! Every entry point that doesn't take an explicit `&dyn CostModel`
29-
//! ([`SketchAlgorithmStrategy::default_cost_model`](crate::replacement::SketchAlgorithmStrategy::default_cost_model),
29+
//! ([`ASAPStrategies::default_cost_model`](crate::replacement::ASAPStrategies::default_cost_model),
3030
//! [`search_workload`](crate::replacement::search_workload)) runs against
3131
//! [`DefaultCostModel`], so a deployment that never plugs in its own cost
3232
//! model keeps today's static-preference-order behavior exactly, byte for
@@ -738,14 +738,14 @@ pub trait CostModel {
738738
/// on [`CandidateLogicalASAPDAGs::cost_sorted`](crate::replacement::CandidateLogicalASAPDAGs::cost_sorted)),
739739
/// not just order candidates against each other — that ordering job
740740
/// already belongs to [`rank_candidates`](Self::rank_candidates) (for a
741-
/// [`SketchAlgorithmStrategy`](crate::replacement::SketchAlgorithmStrategy)
741+
/// [`ASAPStrategies`](crate::replacement::ASAPStrategies)
742742
/// group) and [`cse_share_decision`](Self::cse_share_decision) (for a
743743
/// [`SharedSubDagStrategy`](crate::replacement::SharedSubDagStrategy)
744744
/// group).
745745
///
746746
/// One method covers both candidate shapes this crate ships:
747747
/// `candidate.replacement`'s [`Replacement::SubDag`] from a summary
748-
/// realization (a `SketchAlgorithmStrategy` candidate — the bound node is
748+
/// realization (a `ASAPStrategies` candidate — the bound node is
749749
/// right there, nothing to reconstruct) and the same arm from a rewrite
750750
/// (a `SharedSubDagStrategy` share-vs-recompute candidate — no bound
751751
/// summary of its own, since sharing is a decision about a target
@@ -1030,7 +1030,7 @@ impl CostModel for DefaultCostModel {
10301030
/// second formula:
10311031
///
10321032
/// - A [`ReplacementProvenance::SummaryRealization`] candidate (a
1033-
/// `SketchAlgorithmStrategy` binding): `cse_recompute_cost` (the one-time
1033+
/// `ASAPStrategies` binding): `cse_recompute_cost` (the one-time
10341034
/// structural cost of building `target` at all) plus
10351035
/// `cse_shared_maintenance_cost` of the candidate's own bound family
10361036
/// (a pricier family — a sketch over an exact accumulator, say —

‎crates/asap-aware-mapping/src/explanation.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
//! collapses into a single question this module asks of *that* data instead:
3232
//! **for a given `TargetSubDAG`, does its candidate list contain anything
3333
//! other than the trivial, no-op realization?** A `TargetSubDAG` whose only
34-
//! candidate is "the one thing `SketchAlgorithmStrategy` would have committed
34+
//! candidate is "the one thing `ASAPStrategies` would have committed
3535
//! to anyway, with no alternative" has no optimization to report — that
3636
//! candidate isn't an *opportunity*, it's just the target's existing shape
3737
//! reflected back. A `TargetSubDAG` with more than one candidate (several
@@ -45,7 +45,7 @@
4545
//! - [`ExplanationKind::SketchApproximation`] — the `TargetSubDAG`'s
4646
//! candidate list contains at least one summary-realization [`Replacement::SubDag`] that
4747
//! actually realizes a sketch family (`FieldDataType::Sketch`), i.e.
48-
//! [`SketchAlgorithmStrategy`] found something to offer beyond whatever
48+
//! [`ASAPStrategies`] found something to offer beyond whatever
4949
//! exact/pass-through candidate [`crate::replacement`]'s own
5050
//! `realizations_for_intent` would have committed to on its own.
5151
//! - [`ExplanationKind::CommonSubexpressionReuse`] — the `TargetSubDAG`
@@ -103,7 +103,7 @@
103103
//! [`ReplacementStrategy`] already *is* that extension point, one layer
104104
//! down, and [`explain_replacements_with`]'s own `strategies`
105105
//! parameter is where a caller plugs in a custom one (or a custom
106-
//! `CostModel`, via [`crate::replacement::SketchAlgorithmStrategy::new`]) — the identical spot
106+
//! `CostModel`, via [`crate::replacement::ASAPStrategies::new`]) — the identical spot
107107
//! [`crate::replacement::search_workload_with`] itself exposes.
108108
//!
109109
//! ## Two guarantees the old traversal made, re-verified against the new one
@@ -176,7 +176,7 @@
176176
//! [`ReplacementSubDAG`]: crate::replacement::ReplacementSubDAG
177177
//! [`Replacement`]: crate::replacement::Replacement
178178
//! [`Replacement::SubDag`]: crate::replacement::Replacement::SubDag
179-
//! [`SketchAlgorithmStrategy`]: crate::replacement::SketchAlgorithmStrategy
179+
//! [`ASAPStrategies`]: crate::replacement::ASAPStrategies
180180
//! [`SharedSubDagStrategy`]: crate::replacement::SharedSubDagStrategy
181181
//! [`CandidateLogicalASAPDAGs`]: crate::replacement::CandidateLogicalASAPDAGs
182182
//! [`TargetSubDAGCandidates`]: crate::replacement::TargetSubDAGCandidates
@@ -206,7 +206,7 @@ use crate::replacement::{
206206
pub enum ExplanationKind {
207207
/// A `TargetSubDAG`'s candidate list contains at least one
208208
/// [`Replacement::SubDag`] that realizes a sketch family —
209-
/// [`crate::replacement::SketchAlgorithmStrategy`] found a genuine sketch
209+
/// [`crate::replacement::ASAPStrategies`] found a genuine sketch
210210
/// alternative for this `Aggregate`, beyond whatever exact/pass-through
211211
/// candidate `crate::replacement`'s own `realizations_for_intent` would
212212
/// have committed to on its own.
@@ -273,7 +273,7 @@ pub fn explain_replacements<Id: Display>(
273273
/// instead of [`crate::replacement::default_strategies`] — the extension
274274
/// point for a deployment-specific [`ReplacementStrategy`], or a custom
275275
/// `CostModel` plugged into
276-
/// [`crate::replacement::SketchAlgorithmStrategy::new`] (e.g. via
276+
/// [`crate::replacement::ASAPStrategies::new`] (e.g. via
277277
/// [`crate::replacement::default_strategies_with`]).
278278
///
279279
/// [`ReplacementStrategy`]: crate::replacement::ReplacementStrategy
@@ -839,7 +839,7 @@ mod tests {
839839
let q = agg(vec![2], default_quantile(0.99), metric_scan(&["job"]));
840840
let custom_model = AlwaysDDSketch;
841841
let strategies: Vec<Box<dyn ReplacementStrategy + '_>> = vec![Box::new(
842-
crate::replacement::SketchAlgorithmStrategy::new(&custom_model),
842+
crate::replacement::ASAPStrategies::new(&custom_model),
843843
)];
844844
let findings = explain_replacements_with(vec![("q", q)], &strategies);
845845
assert_eq!(findings.len(), 1);

‎crates/asap-aware-mapping/src/grouping.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
//! An earlier draft of this module (written against the very first draft of
4242
//! #251) reused a `CostModel`-wrapping adapter that "steered" a
4343
//! whole-recursive-bind decision procedure toward a specific `SketchKind`,
44-
//! the same pattern [`crate::replacement::SketchAlgorithmStrategy`]'s own module
44+
//! the same pattern [`crate::replacement::ASAPStrategies`]'s own module
4545
//! docs explain was deliberately deleted from this crate as an anti-pattern:
4646
//! forcing a choice via a whole-tree `CostModel` adapter had a real bug where
4747
//! the forced choice could leak into a target's own nested aggregates. This
@@ -53,7 +53,7 @@
5353
//! passes that exact,
5454
//! already-decided `Realization` to
5555
//! [`crate::replacement::construct_summary`] — the same first-class,
56-
//! one-candidate-at-a-time primitive [`crate::replacement::SketchAlgorithmStrategy`]
56+
//! one-candidate-at-a-time primitive [`crate::replacement::ASAPStrategies`]
5757
//! itself calls once per candidate. No adapter, no steering, no risk of a
5858
//! forced choice leaking into nested aggregates.
5959
//!
@@ -111,15 +111,15 @@ pub fn has_subpopulations(reduction: &Reduction) -> bool {
111111

112112
/// A single static instance so [`HydraGroupingStrategy::default_cost_model`]
113113
/// can hand out a `&'static dyn CostModel` without heap-allocating one — same
114-
/// pattern [`crate::replacement::SketchAlgorithmStrategy`] uses.
114+
/// pattern [`crate::replacement::ASAPStrategies`] uses.
115115
static DEFAULT_COST_MODEL: DefaultCostModel = DefaultCostModel;
116116

117117
/// Wraps the `GroupingStrategy` axis (issue #256) as a
118-
/// [`ReplacementStrategy`]: for a target [`SketchAlgorithmStrategy`](crate::replacement::SketchAlgorithmStrategy)
118+
/// [`ReplacementStrategy`]: for a target [`ASAPStrategies`](crate::replacement::ASAPStrategies)
119119
/// already has an opinion on, offers an additional
120120
/// `GroupingStrategy::SharedMultiSubpopulation` candidate wherever the
121121
/// legality conditions in the module docs above hold — alongside, not
122-
/// instead of, the per-subpopulation candidates `SketchAlgorithmStrategy`
122+
/// instead of, the per-subpopulation candidates `ASAPStrategies`
123123
/// itself enumerates. The workload search composes both strategies over the
124124
/// same target, so it sees every summary-family alternative *and* the Hydra
125125
/// alternative; the built-in workload search registers both strategies, and
@@ -133,7 +133,7 @@ pub struct HydraGroupingStrategy<'a> {
133133
impl HydraGroupingStrategy<'static> {
134134
/// A strategy that ranks/binds via the built-in [`DefaultCostModel`] —
135135
/// what a deployment gets with no custom cost model plugged in, the same
136-
/// default [`crate::replacement::SketchAlgorithmStrategy::default_cost_model`]
136+
/// default [`crate::replacement::ASAPStrategies::default_cost_model`]
137137
/// offers.
138138
pub fn default_cost_model() -> Self {
139139
Self {
@@ -145,7 +145,7 @@ impl HydraGroupingStrategy<'static> {
145145
impl<'a> HydraGroupingStrategy<'a> {
146146
/// A strategy that ranks/binds via `cost_model` instead of the built-in
147147
/// static preference order — the same customization point
148-
/// [`crate::replacement::SketchAlgorithmStrategy::new`] already offers.
148+
/// [`crate::replacement::ASAPStrategies::new`] already offers.
149149
pub fn new(cost_model: &'a dyn CostModel) -> Self {
150150
Self {
151151
planning_inputs: CandidatePlanningInputs::with_default_accuracy(cost_model),
@@ -814,7 +814,7 @@ mod tests {
814814
/// A custom `CostModel` doesn't change *which* candidate is offered —
815815
/// only which sketch candidate `realizations_for_intent` itself would
816816
/// have ranked first, and how that candidate's own params are sized —
817-
/// same guarantee `SketchAlgorithmStrategy` makes for its own candidates.
817+
/// same guarantee `ASAPStrategies` makes for its own candidates.
818818
struct PreferDDSketch;
819819
impl CostModel for PreferDDSketch {
820820
fn rank_candidates(

‎crates/asap-aware-mapping/src/lib.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
//! **Common sub-expression elimination (CSE) is not this crate's job.**
99
//! Detection is a primary pass over the pre-ASAP operator IR itself
1010
//! (`asap_types::ir::cse`, design tracked in issue #223), run before a
11-
//! tree ever reaches [`replacement::SketchAlgorithmStrategy`] — see issue #222
11+
//! tree ever reaches [`replacement::ASAPStrategies`] — see issue #222
1212
//! for why (batch query optimization needs to see shared work across a
1313
//! `QueryWorkload` before summary binding, not after). This crate may
1414
//! eventually run a second, narrower CSE pass of its own over an
@@ -92,7 +92,7 @@
9292
//! #33) is an additional `ReplacementStrategy`: the orthogonal
9393
//! `GroupingStrategy` axis (one summary instance per `by` subpopulation
9494
//! versus one shared Hydra-family structure serving all of them), offered
95-
//! alongside the candidates [`replacement::SketchAlgorithmStrategy`]
95+
//! alongside the candidates [`replacement::ASAPStrategies`]
9696
//! enumerates for the same target.
9797
//! - [`rewrite`] — the "semantic-equivalent rewriting (e.g. `avg` →
9898
//! `sum`/`count`) to increase how often the [sharing/sketch] optimizations
@@ -117,7 +117,7 @@
117117
//! |---|---|---|
118118
//! | Schema resolution | Derive input schemas and resolve column names to positions | `asap_types::pre_asap::SchemaResolver::resolve_schema`, `resolve_root` |
119119
//! | Realization | Enumerate ranked physical forms for one aggregate intent | `replacement::realizations_for_intent` |
120-
//! | Replacement | Construct each candidate summary sub-DAG | [`replacement::SketchAlgorithmStrategy`] |
120+
//! | Replacement | Construct each candidate summary sub-DAG | [`replacement::ASAPStrategies`] |
121121
//! | Search | Enumerate and compare alternatives across a workload | [`replacement::search_workload`] |
122122
//! | Runtime placement | Choose deployment locations and concrete executors | Downstream physical plan providers |
123123
//!
@@ -208,12 +208,12 @@ pub use recurrence::{
208208
};
209209
pub use replacement::{
210210
default_strategies, default_strategies_with, is_logical_rewrite, search_workload,
211-
search_workload_with, search_workload_with_targets, summary_candidates,
211+
search_workload_with, search_workload_with_targets, summary_candidates, ASAPStrategies,
212212
CandidateLogicalASAPDAGs, CompositionDecision, GlobalSelection, Matcher, Proposals,
213213
RankedTargetSubDAGCandidates, Realization, RealizationError, RecurrenceProfileMap,
214214
RejectedCandidate, Replacement, ReplacementProvenance, ReplacementStrategy, ReplacementSubDAG,
215-
SharedSubDagStrategy, SketchAlgorithmStrategy, TargetSubDAG, TargetSubDAGCandidates,
216-
TargetSubDAGSelection, MAX_SEARCH_ITERATIONS,
215+
SharedSubDagStrategy, TargetSubDAG, TargetSubDAGCandidates, TargetSubDAGSelection,
216+
MAX_SEARCH_ITERATIONS,
217217
};
218218
pub use rewrite::{AvgToSumOverCountStrategy, SemanticEquivalentRewriteStrategy};
219219
pub use summary_maintenance_dag_export::{

‎crates/asap-aware-mapping/src/physical_plan_cost_model.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ mod tests {
691691
version: "unused-base-v1".into(),
692692
};
693693
let candidates =
694-
crate::replacement::SketchAlgorithmStrategy::default_cost_model().replacements(&target);
694+
crate::replacement::ASAPStrategies::default_cost_model().replacements(&target);
695695
provider.storage_io = Some(profile.clone());
696696
let model = PhysicalPlanCostModel::new(&provider, base.clone()).unwrap();
697697
let estimate = model.estimate_candidate(&candidates[0], &target).unwrap();
@@ -722,7 +722,7 @@ mod tests {
722722
let root = query();
723723
let target = TargetSubDAG::new(&root);
724724
let candidates =
725-
crate::replacement::SketchAlgorithmStrategy::default_cost_model().replacements(&target);
725+
crate::replacement::ASAPStrategies::default_cost_model().replacements(&target);
726726
let provider = TestProvider::new(true, 800);
727727
let model = PhysicalPlanCostModel::new(&provider, calibration()).unwrap();
728728
let estimate = model.estimate_candidate(&candidates[0], &target).unwrap();
@@ -1019,7 +1019,7 @@ mod tests {
10191019
}
10201020

10211021
let root = query();
1022-
let candidates = crate::replacement::SketchAlgorithmStrategy::default_cost_model()
1022+
let candidates = crate::replacement::ASAPStrategies::default_cost_model()
10231023
.replacements(&TargetSubDAG::new(&root));
10241024
let provider = WrongScope(TestProvider::new(true, 800));
10251025
let model = PhysicalPlanCostModel::new(&provider, calibration()).unwrap();
@@ -1065,7 +1065,7 @@ mod tests {
10651065
}
10661066

10671067
let root = query();
1068-
let candidates = crate::replacement::SketchAlgorithmStrategy::default_cost_model()
1068+
let candidates = crate::replacement::ASAPStrategies::default_cost_model()
10691069
.replacements(&TargetSubDAG::new(&root));
10701070
let model = PhysicalPlanCostModel::new(&BlankVersionProvider, calibration()).unwrap();
10711071
assert_eq!(
@@ -1092,7 +1092,7 @@ mod tests {
10921092
#[test]
10931093
fn sibling_candidates_share_one_scope_and_raw_baseline() {
10941094
let root = query();
1095-
let candidates = crate::replacement::SketchAlgorithmStrategy::default_cost_model()
1095+
let candidates = crate::replacement::ASAPStrategies::default_cost_model()
10961096
.replacements(&TargetSubDAG::new(&root));
10971097
assert!(candidates.len() >= 2);
10981098
let provider = TestProvider::new(true, 800);

0 commit comments

Comments
 (0)