feat: Add problem variants, documentation improvements, and reduction macro#25
feat: Add problem variants, documentation improvements, and reduction macro#25
Conversation
… macro ## Problem Variants - Add `Unweighted` marker type for unweighted problems (like Julia's UnitWeight) - Extend `ReductionEntry` with `source_weighted` and `target_weighted` fields - Generate variant IDs: `ProblemName[/GraphType][/Weighted]` - Update JSON schema with parent, graph_type, and weighted fields - Update Typst diagram to auto-position variant nodes ## Documentation Improvements - Add table of contents to reductions.typ - Add Rust data structures for each problem definition - Add minimal working examples for reduction theorems - Update CLAUDE.md with new patterns and conventions ## Proc Macro - Create `problemreductions-macros` crate with `#[reduction]` attribute - Auto-extract type info from ReduceTo impl signatures - Detect weighted vs unweighted from type parameters - Generate inventory::submit! calls automatically Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #25 +/- ##
==========================================
+ Coverage 97.12% 97.14% +0.02%
==========================================
Files 75 76 +1
Lines 20591 20972 +381
==========================================
+ Hits 19998 20374 +376
- Misses 593 598 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replaces GraphType and Weight associated types with extensible fn variant() method returning key-value attributes. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
17 tasks covering: helper module, trait changes, model updates, registry changes, macro updates, and graph export format. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add utility module for extracting short type names from full Rust type paths. This helper will be used by the Problem trait's variant() method implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Update reduction-diagram.typ to handle new JSON format with structured variants (name + variant object) instead of string IDs - Add variant() method implementations to all problem definitions in reductions.typ to reflect the new Problem trait design - Filter diagram to show only base problems (empty variants) for clarity Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Variant Trait ImplementationThis update replaces the Beforeimpl<W> Problem for IndependentSet<W> {
const NAME: &'static str = "IndependentSet";
type GraphType = SimpleGraph;
type Weight = W;
type Size = W;
// ...
}Afterimpl<W: 'static> Problem for IndependentSet<W> {
const NAME: &'static str = "IndependentSet";
type Size = W;
fn variant() -> Vec<(&'static str, &'static str)> {
vec![
("graph", "SimpleGraph"),
("weight", short_type_name::<W>()),
]
}
// ...
}Key Changes
Benefits
|
Add comprehensive tests for the variant() method across all Problem implementations to improve code coverage: - src/variant.rs: test_variant_for_problems covers IndependentSet, VertexCovering, DominatingSet, Matching, MaxCut, Coloring, MaximalIS, Satisfiability, KSatisfiability, SetPacking, SetCovering, SpinGlass, QUBO, CircuitSAT, Factoring, BicliqueCover, BMF, and PaintShop - src/rules/graph.rs: add tests for variant_to_map, make_variant_ref, and JSON export functions that use variant data Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR implements a comprehensive refactoring to add problem variants to the reduction system. The main changes include:
Changes:
- Problem Variants: Replaces fixed
type GraphTypeandtype Weightassociated types with extensiblefn variant()method that returns key-value attributes. AddsUnweightedmarker type and updatesReductionEntryto track variants. - Proc Macro: Introduces
#[reduction]attribute macro to auto-generateReductionEntryregistrations fromReduceToimpl signatures, reducing boilerplate. - Documentation: Adds table of contents, Rust data structures, and minimal working examples to the technical paper.
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/variant.rs | New helper module for extracting short type names from full paths |
| src/types.rs | Adds Unweighted marker type for unweighted problem variants |
| src/traits.rs | Replaces GraphType/Weight associated types with variant() method |
| src/graph_types.rs | Simplifies GraphMarker trait by removing NAME constant |
| src/rules/registry.rs | Updates ReductionEntry to use variant slices instead of separate fields |
| src/rules/graph.rs | Updates JSON export to use structured variant format with BTreeMap |
| src/models/**/*.rs | Updates ~20 problem implementations to use variant() method |
| src/rules/**/*.rs | Updates reduction files to use #[reduction] macro or new manual format |
| problemreductions-macros/ | New proc macro crate for #[reduction] attribute |
| docs/paper/reductions.typ | Adds TOC, Rust structs, and code examples |
| docs/paper/reduction-diagram.typ | New diagram rendering with variant support |
| docs/paper/reduction_graph.json | Regenerated with new variant structure |
| .claude/CLAUDE.md | Documents new patterns and macro usage |
| Cargo.toml | Adds workspace and macro dependency |
Comments suppressed due to low confidence (1)
src/rules/sat_ksat.rs:572
- The manual registration for SAT -> KSAT uses empty strings for the graph field, but this creates inconsistency. The comment says "generated by macro, can't use #[reduction]", but the actual problem is that the macro would extract "W" and "K" as type parameter names rather than concrete types.
The empty string "" for graph is used here but other problems use "SimpleGraph". This inconsistency propagates to the JSON output where we see both patterns.
Consider either:
- Using "SimpleGraph" consistently for all problems (even non-graph ones) as the default
- Or documenting that empty string is appropriate for problems that don't have a traditional graph structure
- Or having the macro/manual registrations omit the graph field entirely when it's not applicable
The current mix of empty strings and missing fields creates confusion about the intended semantics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| impl ReductionEdge { | ||
| /// Get the graph type from the source variant, or "SimpleGraph" as default. | ||
| pub fn source_graph(&self) -> &'static str { | ||
| self.source_variant | ||
| .iter() | ||
| .find(|(k, _)| *k == "graph") | ||
| .map(|(_, v)| *v) | ||
| .unwrap_or("SimpleGraph") | ||
| } | ||
|
|
||
| /// Get the graph type from the target variant, or "SimpleGraph" as default. | ||
| pub fn target_graph(&self) -> &'static str { | ||
| self.target_variant | ||
| .iter() | ||
| .find(|(k, _)| *k == "graph") | ||
| .map(|(_, v)| *v) | ||
| .unwrap_or("SimpleGraph") | ||
| } | ||
| } |
There was a problem hiding this comment.
The source_graph() and target_graph() helper methods default to "SimpleGraph" when the "graph" key is not found, but they don't handle the case where the key exists but has an empty string value "".
When source_variant contains ("graph", ""), the method will return "" instead of defaulting to "SimpleGraph". This inconsistency means that some problems will report their graph type as an empty string while others report "SimpleGraph".
Consider updating the logic to also treat empty strings as missing:
pub fn source_graph(&self) -> &'static str {
self.source_variant
.iter()
.find(|(k, _)| *k == "graph")
.map(|(_, v)| *v)
.filter(|v| !v.is_empty()) // Add this line
.unwrap_or("SimpleGraph")
}This would make the behavior more consistent with the intended semantics where "SimpleGraph" is the default.
docs/paper/reduction-diagram.typ
Outdated
| if "graph" in n.variant and n.variant.graph != "" and n.variant.graph != "SimpleGraph" { | ||
| suffix.push(n.variant.graph) | ||
| } | ||
| if "weight" in n.variant and n.variant.weight != "" and n.variant.weight != "Unweighted" and n.variant.weight != "W" { |
There was a problem hiding this comment.
The build-node-label function filters out generic type parameter "W" on line 46, but doesn't filter out "K" (used by KSatisfiability). This means nodes with "weight": "K" in the JSON will be displayed with "/K" suffix in the diagram.
The filter should be updated to handle all single-letter generic type parameters consistently:
if "weight" in n.variant and n.variant.weight != "" and n.variant.weight != "Unweighted" and n.variant.weight.len() != 1 {
suffix.push(n.variant.weight)
}Or list all known generic parameters:
if "weight" in n.variant and n.variant.weight != "" and n.variant.weight != "Unweighted" and n.variant.weight != "W" and n.variant.weight != "K" {
suffix.push(n.variant.weight)
}This issue is related to the proc macro extracting generic type parameters as weight names.
| if "weight" in n.variant and n.variant.weight != "" and n.variant.weight != "Unweighted" and n.variant.weight != "W" { | |
| if "weight" in n.variant and n.variant.weight != "" and n.variant.weight != "Unweighted" and n.variant.weight.len() != 1 { |
| fn get_weight_name(ty: &Type) -> String { | ||
| match ty { | ||
| Type::Path(type_path) => { | ||
| type_path | ||
| .path | ||
| .segments | ||
| .last() | ||
| .map(|s| s.ident.to_string()) | ||
| .unwrap_or_else(|| "Unweighted".to_string()) | ||
| } | ||
| _ => "Unweighted".to_string(), | ||
| } | ||
| } |
There was a problem hiding this comment.
The proc macro's get_weight_name() function extracts the literal type name from the AST, which results in generic type parameter names (like "W", "K") appearing in the generated source_variant and target_variant fields.
For example, when processing impl<W> ReduceTo<VertexCovering<W>> for IndependentSet<W>, the macro extracts "W" as the weight type name and generates:
source_variant: &[("graph", "SimpleGraph"), ("weight", "W")],
target_variant: &[("graph", "SimpleGraph"), ("weight", "W")],This is semantically incorrect because "W" is a type parameter, not a concrete weight type. The reduction graph JSON then contains nodes with "weight": "W" and "weight": "K", which are not meaningful variant identifiers.
The macro should either:
- Skip generic type parameters and default to "Unweighted" when encountering single uppercase letters
- Or document that reductions using generic type parameters should manually specify
source_weighted/target_weightedattributes
This affects the generated docs/paper/reduction_graph.json which contains many entries with generic type parameters instead of concrete types.
| "name": "Factoring", | ||
| "variant": {}, | ||
| "category": "specialized" | ||
| }, | ||
| { | ||
| "name": "Factoring", | ||
| "variant": { | ||
| "graph": "", | ||
| "weight": "Unweighted" | ||
| }, | ||
| "category": "specialized" | ||
| }, | ||
| { | ||
| "name": "Factoring", | ||
| "variant": { | ||
| "graph": "SimpleGraph", | ||
| "weight": "Unweighted" | ||
| }, | ||
| "category": "specialized" | ||
| }, | ||
| { | ||
| "id": "ILP", | ||
| "label": "ILP", | ||
| "name": "ILP", | ||
| "variant": {}, | ||
| "category": "optimization" | ||
| }, | ||
| { | ||
| "name": "ILP", | ||
| "variant": { | ||
| "graph": "", | ||
| "weight": "Unweighted" | ||
| }, | ||
| "category": "optimization" | ||
| }, |
There was a problem hiding this comment.
The generated JSON contains duplicate node entries with inconsistent graph field values. For example, Factoring appears with three different variant combinations:
{}(empty variant){"graph": "", "weight": "Unweighted"}{"graph": "SimpleGraph", "weight": "Unweighted"}
The empty string "" for the graph field is inconsistent with using "SimpleGraph" as a value elsewhere. This likely comes from different reduction entries using different values for the graph field when Factoring doesn't operate on a traditional graph structure.
Consider either:
- Standardizing on a specific value (e.g., always use "SimpleGraph" as default or always use empty string for non-graph problems)
- Adding validation to prevent empty strings in variant fields
- Documenting when empty string is appropriate vs when to omit the field entirely
This inconsistency may cause issues in the Typst diagram rendering or graph analysis code that expects consistent formatting.
| /// # Example | ||
| /// | ||
| /// ``` | ||
| /// use problemreductions::types::Unweighted; | ||
| /// | ||
| /// // Problems can be parameterized by weight type: | ||
| /// // - `IndependentSet<Unweighted>` - unweighted (default) | ||
| /// // - `IndependentSet<i32>` - weighted with integer weights | ||
| /// ``` |
There was a problem hiding this comment.
The documentation example suggests using Unweighted as a type parameter:
// - `IndependentSet<Unweighted>` - unweighted (default)
// - `IndependentSet<i32>` - weighted with integer weightsHowever, Unweighted doesn't implement the required numeric traits (Num, Zero, AddAssign) needed by IndependentSet<W>, so IndependentSet<Unweighted> won't actually compile.
The actual usage in the codebase shows that "Unweighted" is only used as a string literal in source_variant/target_variant arrays to indicate an unweighted problem variant, not as an actual generic type parameter.
The documentation should be updated to clarify that:
- Unweighted problems still use
i32or similar as the weight type parameter at runtime - "Unweighted" in the variant system is just metadata indicating all weights are uniform
- Or, alternatively, implement the numeric traits for
Unweightedif it's intended to be used as a real type parameter
| ### Manual Registration (Alternative) | ||
| ```rust | ||
| inventory::submit! { | ||
| ReductionEntry { | ||
| source_name: "SourceProblem", | ||
| target_name: "TargetProblem", | ||
| source_graph: "SimpleGraph", | ||
| target_graph: "SimpleGraph", | ||
| source_weighted: false, | ||
| target_weighted: false, | ||
| overhead_fn: || ReductionOverhead::new(...), | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
The CLAUDE.md documentation shows an outdated manual registration example that uses the old field names:
source_graph: "SimpleGraph",
target_graph: "SimpleGraph",
source_weighted: false,
target_weighted: false,But the actual ReductionEntry structure now uses:
source_variant: &[("graph", "SimpleGraph"), ("weight", "Unweighted")],
target_variant: &[("graph", "SimpleGraph"), ("weight", "Unweighted")],This example should be updated to match the current API to avoid confusing developers who reference this documentation.
Add tests for: - GraphProblem variant() in template.rs - ILP variant() in ilp.rs - NearlyEqualProblem variant() in brute_force.rs - Unweighted methods in types.rs - Test problems' variant() in brute_force.rs These tests improve patch coverage for the variant trait implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add coverage tests for SimpleWeightedProblem, SimpleCsp, and MultiFlavorProblem variant() methods. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Exclude problemreductions-macros from coverage since proc-macro code runs at compile time and cannot be measured by runtime coverage tools like tarpaulin. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add tests for: - Target weighted (source unweighted) - Both source and target weighted These cover additional branches in is_base_reduction() method. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The diagram was filtering for edges between nodes with empty variants, but all edges in the JSON connect nodes with variant attributes. Updated the Typst diagram to: - Filter edges by base problem names (ignoring variants) - Deduplicate edges by source-target pair - Merge bidirectionality for edges that appear in both directions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1. src/rules/graph.rs: source_graph() and target_graph() now treat empty strings as missing and default to "SimpleGraph". JSON export also normalizes empty graph values to "SimpleGraph" for consistency. 2. docs/paper/reduction-diagram.typ: Filter single-letter generic type parameters (W, K, T, etc.) from node labels, not just "W". 3. problemreductions-macros/src/lib.rs: get_weight_name() now treats single uppercase letters as generic type parameters and defaults to "Unweighted" instead of using the literal parameter name. 4. src/types.rs: Updated Unweighted documentation to clarify that it's used as metadata in variant() method, not as an actual type parameter. 5. .claude/CLAUDE.md: Updated manual registration example to use the current source_variant/target_variant field names. 6. Regenerated docs/paper/reduction_graph.json with consistent values. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary
Unweightedmarker type and extendReductionEntryto track graph type and weighted status. Variant nodes are shown in the reduction diagram with hierarchical layout.reductions.typ#[reduction]attribute macro to auto-generateReductionEntryfromReduceToimpl signaturesChanges
Problem Variants
Unweightedmarker type insrc/types.rs(similar to Julia'sUnitWeight)ReductionEntrywithsource_weighted,target_weightedfieldssource_variant_id()/target_variant_id()methods for variant namingparent,graph_type,weightedfieldsDocumentation
docs/paper/reductions.typCLAUDE.mdwith new patternsProc Macro (
problemreductions-macros)#[reduction]attribute macroinventory::submit!automaticallyTest plan
make testpasses (999 tests)make papercompiles successfullymake export-graphgenerates valid JSON🤖 Generated with Claude Code