Skip to content

Commit b0f2cab

Browse files
GiggleLiuclaude
andauthored
refactor: finish generalized aggregation migration (#754)
* refactor: add aggregate value primitives * refactor: unify core solving on aggregate values * refactor: split dynamic value solve from witness solve * refactor: add aggregate reduction execution * refactor: make reduction paths capability-aware * save point 1 * refactor: finish generalized aggregation migration * fix ci * fix: resolve clippy warnings in CLI crate - Inline string literals in eprintln! format strings (print_literal) - Use saturating_sub instead of manual arithmetic check - Merge identical if-else branches in add_ilp_solver_hint Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: migrate new models to aggregation trait hierarchy Adapt ConsecutiveOnesMatrixAugmentation, PartialFeedbackEdgeSet, and GroupingBySwapping (merged from main) to the new Problem trait: - type Metric -> type Value - SatisfactionProblem -> WitnessProblem - Remove unused Solver imports from tests Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * update skills * clean up * update - clean up * fix: remove legacy Valid/Invalid formatting, update stale module descriptions Replace legacy Valid(...)/Invalid display strings with direct aggregate wrapper names (Max, Min, Or, Sum). Update export_module_graph.rs to reflect the new trait hierarchy. Add doc comment on EdgeCapabilities default. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test: improve coverage for aggregate infrastructure Add tests for: - SolveViaReductionError Display/Error impls - ILPSolver::solve_dyn, supports_direct_dyn, try_solve_via_reduction - solve_via_reduction with nonexistent problem - format_metric direct usage - LoadedDynProblem backward-compat solve and Debug - AggregateReductionChain single-step path rejection - Aggregate chain with witness-only edge returns None Re-export SolveViaReductionError from ilp module. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: update CLI tests from Valid(...) to Max(...) format Three CLI tests still asserted stdout.contains("Valid") after the aggregate formatting change. Update to match the new Max(...) output. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * ci: add --workspace to test job for parity with coverage The Test job was running without --workspace, which could miss CLI integration test failures that the Coverage job (which uses --workspace) would catch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * clean up implementation plan --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 5441368 commit b0f2cab

File tree

349 files changed

+7912
-6504
lines changed

Some content is hidden

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

349 files changed

+7912
-6504
lines changed

.claude/CLAUDE.md

Lines changed: 21 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ make release V=x.y.z # Tag and push a new release (CI publishes to crates.io)
8787
- `misc/` - Unique input structures
8888
- Run `pred list` for the full catalog of problems, variants, and reductions; `pred show <name>` for details on a specific problem
8989
- `src/rules/` - Reduction rules + inventory registration
90-
- `src/solvers/` - BruteForce solver, ILP solver (feature-gated). To check if a problem supports ILP solving (via reduction path), run `pred path <ProblemName> ILP`
91-
- `src/traits.rs` - `Problem`, `OptimizationProblem`, `SatisfactionProblem` traits
92-
- `src/rules/traits.rs` - `ReduceTo<T>`, `ReductionResult` traits
90+
- `src/solvers/` - BruteForce solver for aggregate values plus witness recovery when supported, ILP solver (feature-gated, witness-only). To check if a problem supports ILP solving via a witness-capable reduction path, run `pred path <ProblemName> ILP`
91+
- `src/traits.rs` - `Problem` trait
92+
- `src/rules/traits.rs` - `ReduceTo<T>`, `ReduceToAggregate<T>`, `ReductionResult`, `AggregateReductionResult` traits
9393
- `src/registry/` - Compile-time reduction metadata collection
9494
- `problemreductions-cli/` - `pred` CLI tool (separate crate in workspace)
9595
- `src/unit_tests/` - Unit test files (mirroring `src/` structure, referenced via `#[path]`)
@@ -104,36 +104,33 @@ make release V=x.y.z # Tag and push a new release (CI publishes to crates.io)
104104
Problem (core trait — all problems must implement)
105105
106106
├── const NAME: &'static str // e.g., "MaximumIndependentSet"
107-
├── type Metric: Clone // SolutionSize<W> for optimization, bool for satisfaction
107+
├── type Value: Clone // aggregate value: Max/Min/Sum/Or/And/Extremum/...
108108
├── fn dims(&self) -> Vec<usize> // config space: [2, 2, 2] for 3 binary variables
109-
├── fn evaluate(&self, config) -> Metric
109+
├── fn evaluate(&self, config) -> Value
110110
├── fn variant() -> Vec<(&str, &str)> // e.g., [("graph","SimpleGraph"), ("weight","i32")]
111111
├── fn num_variables(&self) -> usize // default: dims().len()
112112
└── fn problem_type() -> ProblemType // catalog bridge: registry lookup by NAME
113+
```
113114

114-
OptimizationProblem : Problem<Metric = SolutionSize<Self::Value>> (extension for optimization)
115-
116-
├── type Value: PartialOrd + Clone // inner objective type (i32, f64, etc.)
117-
└── fn direction(&self) -> Direction // Maximize or Minimize
115+
**Witness-capable objective problems** (e.g., `MaximumIndependentSet`) typically use `Value = Max<W::Sum>`, `Min<W::Sum>`, or `Extremum<W::Sum>`.
118116

119-
SatisfactionProblem : Problem<Metric = bool> (marker trait for decision problems)
120-
```
117+
**Witness-capable feasibility problems** (e.g., `Satisfiability`) typically use `Value = Or`.
121118

122-
**Satisfaction problems** (e.g., `Satisfiability`) use `Metric = bool` and implement `SatisfactionProblem`.
119+
**Aggregate-only problems** use fold values such as `Sum<W>` or `And`; these solve to a value but have no representative witness configuration.
123120

124-
**Optimization problems** (e.g., `MaximumIndependentSet`) use `Metric = SolutionSize<W>` where:
121+
Common aggregate wrappers live in `src/types.rs`:
125122
```rust
126-
enum SolutionSize<T> { Valid(T), Invalid } // Invalid = infeasible config
127-
enum Direction { Maximize, Minimize }
123+
Max<V>, Min<V>, Sum<W>, Or, And, Extremum<V>, ExtremumSense
128124
```
129125

130126
### Key Patterns
131127
- `variant_params!` macro implements `Problem::variant()` — e.g., `crate::variant_params![G, W]` for two type params, `crate::variant_params![]` for none (see `src/variant.rs`)
132-
- `declare_variants!` proc macro registers concrete type instantiations with best-known complexity and registry-backed dynamic dispatch metadata — every entry must specify `opt` or `sat`, and one entry per problem may be marked `default` (see `src/models/graph/maximum_independent_set.rs`). Variable names in complexity strings are validated at compile time against actual getter methods.
128+
- `declare_variants!` proc macro registers concrete type instantiations with best-known complexity and registry-backed load/serialize/value-solve/witness-solve metadata. One entry per problem may be marked `default`, and variable names in complexity strings are validated at compile time against actual getter methods.
133129
- Problems parameterized by graph type `G` and optionally weight type `W` (problem-dependent)
134-
- `ReductionResult` provides `target_problem()` and `extract_solution()`
135-
- `Solver::find_best()``Option<Vec<usize>>` for optimization problems; `Solver::find_satisfying()``Option<Vec<usize>>` for `Metric = bool`
136-
- `BruteForce::find_all_best()` / `find_all_satisfying()` return `Vec<Vec<usize>>` for all optimal/satisfying solutions
130+
- `Solver::solve()` computes the aggregate value for any `Problem` whose `Value` implements `Aggregate`
131+
- `BruteForce::find_witness()` / `find_all_witnesses()` recover witnesses only when `P::Value::supports_witnesses()`
132+
- `ReductionResult` provides `target_problem()` and `extract_solution()` for witness/config workflows; `AggregateReductionResult` provides `extract_value()` for aggregate/value workflows
133+
- CLI-facing dynamic formatting uses aggregate wrapper names directly (for example `Max(2)`, `Min(None)`, `Or(true)`, or `Sum(56)`)
137134
- Graph types: SimpleGraph, PlanarGraph, BipartiteGraph, UnitDiskGraph, KingsSubgraph, TriangularSubgraph
138135
- Weight types: `One` (unit weight marker), `i32`, `f64` — all implement `WeightElement` trait
139136
- `WeightElement` trait: `type Sum: NumericSize` + `fn to_sum(&self)` — converts weight to a summable numeric type
@@ -170,10 +167,12 @@ Reduction graph nodes use variant key-value pairs from `Problem::variant()`:
170167
- Default variant ranking: `SimpleGraph`, `One`, `KN` are considered default values; variants with the most default values sort first
171168
- Nodes come exclusively from `#[reduction]` registrations; natural edges between same-name variants are inferred from the graph/weight subtype partial order
172169
- Each primitive reduction is determined by the exact `(source_variant, target_variant)` endpoint pair
173-
- `#[reduction]` accepts only `overhead = { ... }`
170+
- Reduction edges carry `EdgeCapabilities { witness, aggregate }`; graph search defaults to witness mode, and aggregate mode is available through `ReductionMode::Aggregate`
171+
- `#[reduction]` accepts only `overhead = { ... }` and currently registers witness/config reductions; aggregate-only edges require manual `ReductionEntry` registration with `reduce_aggregate_fn`
174172

175173
### Extension Points
176174
- New models register dynamic load/serialize/brute-force dispatch through `declare_variants!` in the model file, not by adding manual match arms in the CLI
175+
- Aggregate-only models are first-class in `declare_variants!`; aggregate-only reduction edges still need manual `ReductionEntry` wiring because `#[reduction]` only registers witness/config reductions today
177176
- Exact registry dispatch lives in `src/registry/`; alias resolution and partial/default variant resolution live in `problemreductions-cli/src/problem_name.rs`
178177
- `pred create` UX lives in `problemreductions-cli/src/commands/create.rs`
179178
- Canonical paper and CLI examples live in `src/example_db/model_builders.rs` and `src/example_db/rule_builders.rs`
@@ -199,8 +198,8 @@ Reduction graph nodes use variant key-value pairs from `Problem::variant()`:
199198
**Reference implementations — read these first:**
200199
- **Reduction test:** `src/unit_tests/rules/minimumvertexcover_maximumindependentset.rs` — closed-loop pattern
201200
- **Model test:** `src/unit_tests/models/graph/maximum_independent_set.rs` — evaluation, serialization
202-
- **Solver test:** `src/unit_tests/solvers/brute_force.rs``find_best` + `find_satisfying`
203-
- **Trait definitions:** `src/traits.rs` (`Problem`, `OptimizationProblem`), `src/solvers/mod.rs` (`Solver`)
201+
- **Solver test:** `src/unit_tests/solvers/brute_force.rs`aggregate `solve()` plus witness recovery helpers
202+
- **Trait definitions:** `src/traits.rs` (`Problem`), `src/solvers/mod.rs` (`Solver`)
204203

205204
### Coverage
206205

.claude/skills/add-model/SKILL.md

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,16 @@ Before any implementation, collect all required information. If called from `iss
1717
|---|------|-------------|---------|
1818
| 1 | **Problem name** | Struct name with optimization prefix | `MaximumClique`, `MinimumDominatingSet` |
1919
| 2 | **Mathematical definition** | Formal definition with objective/constraints | "Given graph G=(V,E), find max-weight subset S where all pairs in S are adjacent" |
20-
| 3 | **Problem type** | Optimization (maximize/minimize) or satisfaction | Optimization (Maximize) |
20+
| 3 | **Problem type** | Objective (`Max`/`Min`), witness (`bool`), or aggregate-only (`Sum`/`And`/custom `Aggregate`) | Objective (Maximize) |
2121
| 4 | **Type parameters** | Graph type `G`, weight type `W`, or other | `G: Graph`, `W: WeightElement` |
2222
| 5 | **Struct fields** | What the struct holds | `graph: G`, `weights: Vec<W>` |
2323
| 6 | **Configuration space** | What `dims()` returns | `vec![2; num_vertices]` for binary vertex selection |
2424
| 7 | **Feasibility check** | How to validate a configuration | "All selected vertices must be pairwise adjacent" |
25-
| 8 | **Objective function** | How to compute the metric | "Sum of weights of selected vertices" |
25+
| 8 | **Per-configuration value** | How `evaluate()` computes the aggregate contribution | "Return `Max(Some(total_weight))` for feasible configs" |
2626
| 9 | **Best known exact algorithm** | Complexity with variable definitions | "O(1.1996^n) by Xiao & Nagamochi (2017), where n = \|V\|" |
2727
| 10 | **Solving strategy** | How it can be solved | "BruteForce works; ILP reduction available" |
2828
| 11 | **Category** | Which sub-module under `src/models/` | `graph`, `formula`, `set`, `algebraic`, `misc` |
29-
| 12 | **Expected outcome from the issue** | Concrete outcome for the issue's example instance | Optimization: one optimal solution + optimal value. Satisfaction: one valid/satisfying solution + why it is valid |
29+
| 12 | **Expected outcome from the issue** | Concrete outcome for the issue's example instance | Objective: one optimal solution + optimal value. Witness: one valid/satisfying solution + why it is valid. Aggregate-only: the final aggregate value and how it is derived |
3030

3131
If any item is missing, ask the user to provide it. Do NOT proceed until the checklist is complete.
3232

@@ -61,7 +61,7 @@ Read these first to understand the patterns:
6161
- **Optimization problem:** `src/models/graph/maximum_independent_set.rs`
6262
- **Satisfaction problem:** `src/models/formula/sat.rs`
6363
- **Model tests:** `src/unit_tests/models/graph/maximum_independent_set.rs`
64-
- **Trait definitions:** `src/traits.rs` (`Problem`, `OptimizationProblem`, `SatisfactionProblem`)
64+
- **Trait definitions / aggregate types:** `src/traits.rs` (`Problem`), `src/types.rs` (`Aggregate`, `Max`, `Min`, `Sum`, `Or`, `And`, `Extremum`)
6565
- **Registry dispatch boundary:** `src/registry/mod.rs`, `src/registry/variant.rs`
6666
- **CLI aliases:** `problemreductions-cli/src/problem_name.rs`
6767
- **CLI creation:** `problemreductions-cli/src/commands/create.rs`
@@ -71,7 +71,8 @@ Read these first to understand the patterns:
7171

7272
Before implementing, make sure the plan explicitly covers these items that structural review checks later:
7373
- `ProblemSchemaEntry` metadata is complete for the current schema shape (`display_name`, `aliases`, `dimensions`, and constructor-facing `fields`)
74-
- `declare_variants!` is present with the correct `opt`/`sat` marker and exactly one `default` variant when multiple concrete variants exist
74+
- `Problem::Value` uses the correct aggregate wrapper and witness support is intentional
75+
- `declare_variants!` is present with exactly one `default` variant when multiple concrete variants exist
7576
- CLI discovery and `pred create <ProblemName>` support are included where applicable
7677
- A canonical model example is registered for example-db / `pred create --example`
7778
- `docs/paper/reductions.typ` adds both the display-name dictionary entry and the `problem-def(...)`
@@ -110,34 +111,32 @@ Create `src/models/<category>/<name>.rs`:
110111
// 1. inventory::submit! for ProblemSchemaEntry
111112
// 2. Struct definition with #[derive(Debug, Clone, Serialize, Deserialize)]
112113
// 3. Constructor (new) + accessor methods
113-
// 4. Problem trait impl (NAME, Metric, dims, evaluate, variant)
114-
// 5. OptimizationProblem or SatisfactionProblem impl
115-
// 6. #[cfg(test)] #[path = "..."] mod tests;
114+
// 4. Problem trait impl (NAME, Value, dims, evaluate, variant)
115+
// 5. #[cfg(test)] #[path = "..."] mod tests;
116116
```
117117

118118
Key decisions:
119119
- **Schema metadata:** `ProblemSchemaEntry` must reflect the current registry schema shape, including `display_name`, `aliases`, `dimensions`, and constructor-facing `fields`
120-
- **Optimization problems:** `type Metric = SolutionSize<W::Sum>`, implement `OptimizationProblem` with `direction()`
121-
- **Satisfaction problems:** `type Metric = bool`, implement `SatisfactionProblem` (marker trait)
120+
- **Objective problems:** use `type Value = Max<_>`, `Min<_>`, or `Extremum<_>` when the model should expose optimization-style witness helpers
121+
- **Witness problems:** use `type Value = Or` for existential feasibility problems
122+
- **Aggregate-only problems:** use a value-only aggregate such as `Sum<_>`, `And`, or a custom `Aggregate` when witnesses are not meaningful
122123
- **Weight management:** use inherent methods (`weights()`, `set_weights()`, `is_weighted()`), NOT traits
123124
- **`dims()`:** returns the configuration space dimensions (e.g., `vec![2; n]` for binary variables)
124-
- **`evaluate()`:** must check feasibility first, then compute objective
125+
- **`evaluate()`:** must return the per-configuration aggregate value. For models with invalid configs, check feasibility first and return the appropriate invalid/false contribution
125126
- **`variant()`:** use the `variant_params!` macro — e.g., `crate::variant_params![G, W]` for `Problem<G, W>`, or `crate::variant_params![]` for problems with no type parameters. Each type parameter must implement `VariantParam` (already done for standard types like `SimpleGraph`, `i32`, `One`). See `src/variant.rs`.
127+
- **Solve surface:** `Solver::solve()` always computes the aggregate value. `pred solve problem.json` prints a `Solution` only when a witness exists; `pred solve bundle.json` and `--solver ilp` remain witness-only workflows
126128

127129
## Step 2.5: Register variant complexity
128130

129131
Add `declare_variants!` at the bottom of the model file (after the trait impls, before the test link). Each line declares a concrete type instantiation with its best-known worst-case complexity:
130132

131133
```rust
132134
crate::declare_variants! {
133-
opt ProblemName<SimpleGraph, i32> => "1.1996^num_vertices",
134-
default opt ProblemName<SimpleGraph, One> => "1.1996^num_vertices",
135+
ProblemName<SimpleGraph, i32> => "1.1996^num_vertices",
136+
default ProblemName<SimpleGraph, One> => "1.1996^num_vertices",
135137
}
136138
```
137139

138-
- Each entry must include an explicit solver kind:
139-
- `opt` for optimization problems (`BruteForce::find_best`)
140-
- `sat` for satisfaction problems (`BruteForce::find_satisfying`)
141140
- Mark exactly one concrete variant `default` when the problem has multiple registered variants
142141
- The complexity string references the getter method names from Step 1.5 (e.g., `num_vertices`) — variable names are validated at compile time against actual getters, so typos cause compile errors
143142
- One entry per supported `(graph, weight)` combination
@@ -146,6 +145,8 @@ crate::declare_variants! {
146145
- A compiled `complexity_eval_fn` plus registry-backed load/serialize/solve dispatch metadata are auto-generated alongside the symbolic expression
147146
- See `src/models/graph/maximum_independent_set.rs` for the reference pattern
148147

148+
`declare_variants!` now handles objective, witness-capable, and aggregate-only models uniformly. Use manual `VariantEntry` wiring only for unusual dynamic-registration work, not for ordinary models.
149+
149150
## Step 3: Register the model
150151

151152
Update these files to register the new problem type:
@@ -160,7 +161,6 @@ The CLI now loads, serializes, and brute-force solves problems through the core
160161

161162
1. **Registry-backed dispatch comes from `declare_variants!`:**
162163
- Make sure every concrete variant you want the CLI to load is listed in `declare_variants!`
163-
- Use the correct `opt`/`sat` marker per entry
164164
- Mark the intended default variant with `default` when applicable
165165

166166
2. **`problemreductions-cli/src/problem_name.rs`:**
@@ -200,9 +200,9 @@ Create `src/unit_tests/models/<category>/<name>.rs`:
200200
Every model needs **at least 3 test functions** (the structural reviewer enforces this). Choose from the coverage areas below — pick whichever are relevant to the model:
201201

202202
- **Creation/basic** — exercise constructor inputs, key accessors, `dims()` / `num_variables()`.
203-
- **Evaluation** — valid and invalid configs so the feasibility boundary is explicit.
204-
- **Direction** — verify optimization direction (optimization problems only).
205-
- **Solver** — brute-force solver finds correct solutions (when the model is small enough).
203+
- **Evaluation** — valid and invalid configs so the feasibility boundary or aggregate contribution is explicit.
204+
- **Direction / sense** — verify runtime optimization sense only for models that use `Extremum<_>`.
205+
- **Solver** — brute-force `solve()` returns the correct aggregate value; if witnesses are supported, verify `find_witness()` / `find_all_witnesses()` as well.
206206
- **Serialization** — round-trip serde (when the model is used in CLI/example-db flows).
207207
- **Paper example** — verify the worked example from the paper entry (see below).
208208

@@ -280,9 +280,11 @@ Structural and quality review is handled by the `review-pipeline` stage, not her
280280
| Forgetting `inventory::submit!` | Every problem needs a `ProblemSchemaEntry` registration |
281281
| Missing `#[path]` test link | Add `#[cfg(test)] #[path = "..."] mod tests;` at file bottom |
282282
| Wrong `dims()` | Must match the actual configuration space (e.g., `vec![2; n]` for binary) |
283+
| Using the wrong aggregate wrapper | Objective models use `Max` / `Min` / `Extremum`, witness models use `bool`, aggregate-only models use a fold value like `Sum` / `And` |
283284
| Not registering in `mod.rs` | Must update both `<category>/mod.rs` and `models/mod.rs` |
284285
| Forgetting `declare_variants!` | Required for variant complexity metadata and registry-backed load/serialize/solve dispatch |
285-
| Wrong `declare_variants!` syntax | Every entry now needs `opt` or `sat`; one entry per problem may be marked `default` |
286+
| Wrong aggregate wrapper | Use `Max` / `Min` / `Extremum` for objective problems, `Or` for existential witness problems, and `Sum` / `And` (or a custom aggregate) for value-only folds |
287+
| Wrong `declare_variants!` syntax | Entries no longer use `opt` / `sat`; one entry per problem may be marked `default` |
286288
| Forgetting CLI alias | Must add lowercase entry in `problem_name.rs` `resolve_alias()` |
287289
| Inventing short aliases | Only use well-established literature abbreviations (MIS, SAT, TSP); do NOT invent new ones |
288290
| Forgetting CLI create | Must add creation handler in `commands/create.rs` and flags in `cli.rs` |

0 commit comments

Comments
 (0)