Skip to content

Commit 1731fbd

Browse files
GiggleLiuclaude
andauthored
test: add coverage for new Expr display/eval and model size getters; fix docs (#100)
- Add 12 tests for Expr: fractional display, Log/Sqrt display, Mul/Pow parenthesization, missing variable eval, scale, ops::Add, substitute and variables for Exp/Log/Sqrt branches - Add test_size_getters to all 19 model test files covering new inherent getter methods (num_vertices, num_edges, num_vars, num_constraints, num_interactions, num_literals, universe_size, m, n, num_variables, num_bits_first, num_bits_second, num_sequence, rank) - Update docs/src/design.md: replace poly!() examples with Expr syntax - Update skills (add-model, add-rule, review-implementation): remove references to deleted problem_size_names/problem_size_values methods - Update issue templates: reference inherent getter methods Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f1d9cc9 commit 1731fbd

25 files changed

Lines changed: 309 additions & 40 deletions

File tree

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ Create `src/models/<category>/<name>.rs`:
5757
// 1. inventory::submit! for ProblemSchemaEntry
5858
// 2. Struct definition with #[derive(Debug, Clone, Serialize, Deserialize)]
5959
// 3. Constructor (new) + accessor methods
60-
// 4. Problem trait impl (NAME, Metric, dims, evaluate, variant, problem_size_names, problem_size_values)
60+
// 4. Problem trait impl (NAME, Metric, dims, evaluate, variant)
6161
// 5. OptimizationProblem or SatisfactionProblem impl
6262
// 6. #[cfg(test)] #[path = "..."] mod tests;
6363
```

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ Before any implementation, collect all required information. If called from `iss
2020
| 3 | **Reduction algorithm** | How to transform source instance to target | "Copy graph and weights; IS on same graph as VC" |
2121
| 4 | **Solution extraction** | How to map target solution back to source | "Complement: `1 - x` for each variable" |
2222
| 5 | **Correctness argument** | Why the reduction preserves optimality | "S is independent set iff V\S is vertex cover" |
23-
| 6 | **Size overhead** | How target size relates to source size | `num_vertices: poly!(num_vertices), num_edges: poly!(num_edges)` |
23+
| 6 | **Size overhead** | How target size relates to source size | `num_vertices = "num_vertices", num_edges = "num_edges"` |
2424
| 7 | **Concrete example** | A small worked-out instance (tutorial style, clear intuition) | "Triangle graph: VC={0,1} -> IS={2}" |
2525
| 8 | **Solving strategy** | How to solve the target problem | "BruteForce, or existing ILP reduction" |
2626
| 9 | **Reference** | Paper, textbook, or URL for the reduction | URL or citation |
@@ -75,13 +75,9 @@ impl ReductionResult for ReductionXToY {
7575

7676
**ReduceTo with `#[reduction]` macro:**
7777
```rust
78-
#[reduction(
79-
overhead = {
80-
ReductionOverhead::new(vec![
81-
("field_name", poly!(source_field)),
82-
])
83-
}
84-
)]
78+
#[reduction(overhead = {
79+
field_name = "source_field",
80+
})]
8581
impl ReduceTo<TargetType> for SourceType {
8682
type Result = ReductionXToY;
8783
fn reduce_to(&self) -> Self::Result { ... }
@@ -180,7 +176,7 @@ Adding a reduction rule does NOT require CLI changes -- the reduction graph is a
180176
| Mistake | Fix |
181177
|---------|-----|
182178
| Forgetting `#[reduction(...)]` macro | Required for compile-time registration in the reduction graph |
183-
| Wrong overhead polynomial | Must accurately reflect the size relationship |
179+
| Wrong overhead expression | Must accurately reflect the size relationship |
184180
| Missing `extract_solution` mapping state | Store any index maps needed in the ReductionResult struct |
185181
| Example missing `pub fn run()` | Required for the test harness (`include!` pattern) |
186182
| Not registering example in `tests/suites/examples.rs` | Must add both `example_test!` and `example_fn!` |

.claude/skills/review-implementation/SKILL.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,12 +88,12 @@ Read the implementation files and assess:
8888
### For Models:
8989
1. **`evaluate()` correctness** -- Does it check feasibility before computing the objective? Does it return `SolutionSize::Invalid` / `false` for infeasible configs?
9090
2. **`dims()` correctness** -- Does it return the actual configuration space? (e.g., `vec![2; n]` for binary)
91-
3. **`problem_size_names`/`problem_size_values` consistency** -- Do the names match what `ReductionOverhead` uses?
91+
3. **Size getter consistency** -- Do the inherent getter methods (e.g., `num_vertices()`, `num_edges()`) match names used in overhead expressions?
9292
4. **Weight handling** -- Are weights managed via inherent methods, not traits?
9393

9494
### For Rules:
9595
1. **`extract_solution` correctness** -- Does it correctly invert the reduction? Does the returned solution have the right length (source dimensions)?
96-
2. **Overhead accuracy** -- Does `poly!(...)` reflect the actual size relationship?
96+
2. **Overhead accuracy** -- Does the `overhead = { field = "expr" }` reflect the actual size relationship?
9797
3. **Example quality** -- Is it tutorial-style? Does it use the instance from the issue? Does the JSON export include both source and target data?
9898
4. **Paper quality** -- Is the reduction-rule statement precise? Is the proof sketch sound? Is the example figure clear?
9999

.github/ISSUE_TEMPLATE/problem.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ Connect fields to the symbols defined above.
5454

5555
<!--
5656
Size metrics characterize instance complexity and are used for reduction overhead analysis.
57-
List the named fields returned by `problem_size_names()` / `problem_size_values()`.
57+
List the named getter methods (e.g., num_vertices(), num_edges()) that the problem type provides.
5858
Use symbols defined above.
5959
6060
Examples:

.github/ISSUE_TEMPLATE/rule.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,9 +26,9 @@ Solution extraction follows from the variable mapping, no need to describe separ
2626

2727
## Size Overhead
2828

29-
<!-- How large is the target instance as a polynomial of the source size?
29+
<!-- How large is the target instance relative to the source size?
3030
Use the symbols defined in the Reduction Algorithm above.
31-
Also provide the code-level metric name (from the problem's `problem_size()` method). -->
31+
Also provide the code-level metric name (matching the problem's inherent getter methods, e.g., num_vertices, num_edges). -->
3232

3333
| Target metric (code name) | Polynomial (using symbols above) |
3434
|----------------------------|----------------------------------|

docs/src/design.md

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,10 @@ impl<W: WeightElement + VariantParam> ReductionResult for ReductionISToVC<W> {
184184
The `#[reduction]` attribute on the `ReduceTo<T>` impl registers the reduction in the global registry (via `inventory`):
185185

186186
```rust,ignore
187-
#[reduction(
188-
overhead = {
189-
ReductionOverhead::new(vec![
190-
("num_vertices", poly!(num_vertices)),
191-
("num_edges", poly!(num_edges)),
192-
])
193-
}
194-
)]
187+
#[reduction(overhead = {
188+
num_vertices = "num_vertices",
189+
num_edges = "num_edges",
190+
})]
195191
impl ReduceTo<MinimumVertexCover<SimpleGraph, i32>>
196192
for MaximumIndependentSet<SimpleGraph, i32>
197193
{
@@ -212,10 +208,12 @@ inventory::submit! {
212208
target_name: "MinimumVertexCover",
213209
source_variant_fn: || <MaximumIndependentSet<SimpleGraph, i32> as Problem>::variant(),
214210
target_variant_fn: || <MinimumVertexCover<SimpleGraph, i32> as Problem>::variant(),
215-
overhead_fn: || ReductionOverhead::new(vec![
216-
("num_vertices", poly!(num_vertices)),
217-
("num_edges", poly!(num_edges)),
218-
]),
211+
overhead_fn: || ReductionOverhead {
212+
output_size: vec![
213+
("num_vertices", Expr::Var("num_vertices")),
214+
("num_edges", Expr::Var("num_edges")),
215+
],
216+
},
219217
module_path: module_path!(),
220218
reduce_fn: |src: &dyn Any| -> Box<dyn DynReductionResult> {
221219
let src = src.downcast_ref::<MaximumIndependentSet<SimpleGraph, i32>>().unwrap();
@@ -296,23 +294,17 @@ For full type control, you can also chain `ReduceTo::reduce_to()` calls manually
296294
<details>
297295
<summary>Overhead evaluation</summary>
298296

299-
Each reduction declares how the output problem size relates to the input, expressed as polynomials. The `poly!` macro provides concise syntax:
297+
Each reduction declares how the output problem size relates to the input, expressed as symbolic `Expr` expressions. The `#[reduction]` macro parses overhead strings at compile time:
300298

301299
```rust,ignore
302-
poly!(num_vertices) // p(x) = num_vertices
303-
poly!(num_vertices ^ 2) // p(x) = num_vertices²
304-
poly!(3 * num_edges) // p(x) = 3 × num_edges
305-
poly!(num_vertices * num_edges) // p(x) = num_vertices × num_edges
300+
#[reduction(overhead = {
301+
num_vars = "num_vertices + num_edges",
302+
num_clauses = "3 * num_edges",
303+
})]
304+
impl ReduceTo<Target> for Source { ... }
306305
```
307306

308-
A `ReductionOverhead` pairs output field names with their polynomials:
309-
310-
```rust,ignore
311-
ReductionOverhead::new(vec![
312-
("num_vars", poly!(num_vertices) + poly!(num_edges)),
313-
("num_clauses", poly!(3 * num_edges)),
314-
])
315-
```
307+
Expressions support: constants, variables, `+`, `*`, `^`, `exp()`, `log()`, `sqrt()`. Each problem type provides inherent getter methods (e.g., `num_vertices()`, `num_edges()`) that the overhead expressions reference.
316308

317309
`evaluate_output_size(input)` substitutes input values:
318310

src/unit_tests/expr.rs

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,4 +141,108 @@ fn test_expr_is_polynomial() {
141141
assert!(Expr::pow(Expr::Var("n"), Expr::Const(2.0)).is_polynomial());
142142
assert!(!Expr::Exp(Box::new(Expr::Var("n"))).is_polynomial());
143143
assert!(!Expr::Log(Box::new(Expr::Var("n"))).is_polynomial());
144+
assert!(!Expr::Sqrt(Box::new(Expr::Var("n"))).is_polynomial());
145+
}
146+
147+
#[test]
148+
fn test_expr_display_fractional_constant() {
149+
assert_eq!(format!("{}", Expr::Const(2.75)), "2.75");
150+
assert_eq!(format!("{}", Expr::Const(0.5)), "0.5");
151+
}
152+
153+
#[test]
154+
fn test_expr_display_log() {
155+
let e = Expr::Log(Box::new(Expr::Var("n")));
156+
assert_eq!(format!("{e}"), "log(n)");
157+
}
158+
159+
#[test]
160+
fn test_expr_display_sqrt() {
161+
let e = Expr::Sqrt(Box::new(Expr::Var("n")));
162+
assert_eq!(format!("{e}"), "sqrt(n)");
163+
}
164+
165+
#[test]
166+
fn test_expr_display_mul_with_add_parenthesization() {
167+
// (a + b) * c should parenthesize the left side
168+
let e = Expr::mul(Expr::add(Expr::Var("a"), Expr::Var("b")), Expr::Var("c"));
169+
assert_eq!(format!("{e}"), "(a + b) * c");
170+
171+
// c * (a + b) should parenthesize the right side
172+
let e = Expr::mul(Expr::Var("c"), Expr::add(Expr::Var("a"), Expr::Var("b")));
173+
assert_eq!(format!("{e}"), "c * (a + b)");
174+
175+
// (a + b) * (c + d) should parenthesize both sides
176+
let e = Expr::mul(
177+
Expr::add(Expr::Var("a"), Expr::Var("b")),
178+
Expr::add(Expr::Var("c"), Expr::Var("d")),
179+
);
180+
assert_eq!(format!("{e}"), "(a + b) * (c + d)");
181+
}
182+
183+
#[test]
184+
fn test_expr_display_pow_with_complex_base() {
185+
// (a + b)^2
186+
let e = Expr::pow(Expr::add(Expr::Var("a"), Expr::Var("b")), Expr::Const(2.0));
187+
assert_eq!(format!("{e}"), "(a + b)^2");
188+
189+
// (a * b)^2
190+
let e = Expr::pow(Expr::mul(Expr::Var("a"), Expr::Var("b")), Expr::Const(2.0));
191+
assert_eq!(format!("{e}"), "(a * b)^2");
192+
}
193+
194+
#[test]
195+
fn test_expr_eval_missing_variable() {
196+
// Missing variable should default to 0
197+
let e = Expr::Var("missing");
198+
let size = ProblemSize::new(vec![("other", 5)]);
199+
assert_eq!(e.eval(&size), 0.0);
200+
}
201+
202+
#[test]
203+
fn test_expr_scale() {
204+
let e = Expr::Var("n").scale(3.0);
205+
let size = ProblemSize::new(vec![("n", 5)]);
206+
assert_eq!(e.eval(&size), 15.0);
207+
}
208+
209+
#[test]
210+
fn test_expr_ops_add_trait() {
211+
let a = Expr::Var("a");
212+
let b = Expr::Var("b");
213+
let e = a + b; // uses std::ops::Add
214+
let size = ProblemSize::new(vec![("a", 3), ("b", 4)]);
215+
assert_eq!(e.eval(&size), 7.0);
216+
}
217+
218+
#[test]
219+
fn test_expr_substitute_exp_log_sqrt() {
220+
let replacement = Expr::Const(2.0);
221+
let mut mapping = HashMap::new();
222+
mapping.insert("n", &replacement);
223+
224+
let e = Expr::Exp(Box::new(Expr::Var("n")));
225+
let result = e.substitute(&mapping);
226+
let size = ProblemSize::new(vec![]);
227+
assert!((result.eval(&size) - 2.0_f64.exp()).abs() < 1e-10);
228+
229+
let e = Expr::Log(Box::new(Expr::Var("n")));
230+
let result = e.substitute(&mapping);
231+
assert!((result.eval(&size) - 2.0_f64.ln()).abs() < 1e-10);
232+
233+
let e = Expr::Sqrt(Box::new(Expr::Var("n")));
234+
let result = e.substitute(&mapping);
235+
assert!((result.eval(&size) - 2.0_f64.sqrt()).abs() < 1e-10);
236+
}
237+
238+
#[test]
239+
fn test_expr_variables_exp_log_sqrt() {
240+
let e = Expr::Exp(Box::new(Expr::Var("a")));
241+
assert_eq!(e.variables(), HashSet::from(["a"]));
242+
243+
let e = Expr::Log(Box::new(Expr::Var("b")));
244+
assert_eq!(e.variables(), HashSet::from(["b"]));
245+
246+
let e = Expr::Sqrt(Box::new(Expr::Var("c")));
247+
assert_eq!(e.variables(), HashSet::from(["c"]));
144248
}

src/unit_tests/models/graph/kcoloring.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,3 +188,10 @@ fn test_is_valid_solution() {
188188
// Invalid: adjacent vertices 0 and 1 have same color
189189
assert!(!problem.is_valid_solution(&[0, 0, 1]));
190190
}
191+
192+
#[test]
193+
fn test_size_getters() {
194+
let problem = KColoring::<K3, _>::new(SimpleGraph::new(3, vec![(0, 1), (1, 2)]));
195+
assert_eq!(problem.num_vertices(), 3);
196+
assert_eq!(problem.num_edges(), 2);
197+
}

src/unit_tests/models/graph/max_cut.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,3 +140,10 @@ fn test_cut_size_method() {
140140
// All same partition: no edges cut
141141
assert_eq!(problem.cut_size(&[0, 0, 0]), 0);
142142
}
143+
144+
#[test]
145+
fn test_size_getters() {
146+
let problem = MaxCut::new(SimpleGraph::new(3, vec![(0, 1), (1, 2)]), vec![1i32; 2]);
147+
assert_eq!(problem.num_vertices(), 3);
148+
assert_eq!(problem.num_edges(), 2);
149+
}

src/unit_tests/models/graph/maximal_is.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -173,3 +173,13 @@ fn test_is_valid_solution() {
173173
// Invalid: {0} is independent but not maximal (vertex 2 can be added)
174174
assert!(!problem.is_valid_solution(&[1, 0, 0]));
175175
}
176+
177+
#[test]
178+
fn test_size_getters() {
179+
let problem = MaximalIS::new(
180+
SimpleGraph::new(4, vec![(0, 1), (1, 2), (2, 3)]),
181+
vec![1i32; 4],
182+
);
183+
assert_eq!(problem.num_vertices(), 4);
184+
assert_eq!(problem.num_edges(), 3);
185+
}

0 commit comments

Comments
 (0)