Simplify variant system and clean up type hierarchy#66
Conversation
… indices in JSON - Replace hardcoded `categorize_type` and `compute_doc_path` with derivation from `ProblemSchemaEntry.module_path` via inventory lookup - Simplify reduction_graph.json edges to use node indices instead of full objects - Use `G::NAME` instead of `short_type_name::<G>()` in variant() for correct graph type names (e.g. "GridGraph" not "GridGraph<i32>") - Remove manual `register_types` function; use `Problem::NAME` directly - Add MIS GridGraph/UnitDiskGraph reductions via unitdiskmapping module - Update docs (introduction.md, reductions.typ) to dereference node indices Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…s concrete The proc macro had a fallback path for type-generic impls that silently hardcoded "SimpleGraph"/"Unweighted" defaults and used heuristics, producing wrong variants (e.g., KColoring lost "k" field, MaximumSetPacking got spurious "graph" field). Changes: - Macro: replace type-generic fallback with compile error, remove source_graph/target_graph/source_weighted/target_weighted attributes, delete dead helper functions - Make 10 ReduceTo impls concrete across 7 reduction files (i32 or f64) - MaximumSetPacking→QUBO: only f64 impl (i32 promotes via natural edge) - Add ConcreteVariantEntry registrations for Unweighted variants so natural weight-promotion edges work correctly Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Problem) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…isfactionProblem) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…/NumericWeight Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace verbose trait bounds (Clone + Default + PartialOrd + Num + Zero + AddAssign + 'static) with WeightElement in Problem and OptimizationProblem impls for all 8 graph problems. Use W::Sum as the metric/value type and W::to_sum() for weight accumulation. Also update 5 reduction rule files that reference these graph problems. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #66 +/- ##
==========================================
+ Coverage 97.01% 97.08% +0.06%
==========================================
Files 178 186 +8
Lines 25540 25643 +103
==========================================
+ Hits 24778 24895 +117
+ Misses 762 748 -14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR refactors the library’s variant/type system by (1) simplifying variant/category metadata derivation (from module_path and node indices in the exported reduction graph JSON) and (2) cleaning up the weight + problem trait hierarchy (introducing WeightElement/One and SatisfactionProblem). It also updates docs, examples, and test fixtures to match the new schema and exports.
Changes:
- Replace
Unweighted/Weights/NumericWeightwithOne+WeightElementand propagate the new metric (W::Sum) across models, rules, tests, and exports. - Export a smaller
reduction_graph.jsonby using node indices for edges and deriving category/doc links fromProblemSchemaEntry.module_path; update docs/visualization consumers accordingly. - Make
#[reduction]registrations require concreteReduceToimpls (no type generics), and add/adjust concrete reductions (incl. MIS→GridGraph).
Reviewed changes
Copilot reviewed 68 out of 72 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/data/qubo/minimumvertexcover_to_qubo.json | Update fixture problem name to match renamed/standardized model naming. |
| tests/data/qubo/maximumsetpacking_to_qubo.json | Update fixture problem name to match renamed/standardized model naming. |
| tests/data/qubo/maximumindependentset_to_qubo.json | Update fixture problem name to match renamed/standardized model naming. |
| src/unit_tests/variant.rs | Update variant expectations to reflect reduced/changed variant keys and default handling. |
| src/unit_tests/types.rs | Replace Unweighted/Weights tests with One/WeightElement tests. |
| src/unit_tests/rules/travelingsalesman_ilp.rs | Formatting + updated constructors/expectations after type-system changes. |
| src/unit_tests/rules/sat_ksat.rs | Update test to target KSatisfiability<3> and clause-size assertions. |
| src/unit_tests/rules/registry.rs | Update registry tests to use One in variants. |
| src/unit_tests/rules/maximumsetpacking_qubo.rs | Switch test instances to MaximumSetPacking<f64> for QUBO reduction. |
| src/unit_tests/rules/maximumindependentset_gridgraph.rs | New unit tests for MIS reductions to GridGraph. |
| src/unit_tests/rules/graph.rs | Update tests for JSON edge indexing, module_path-derived category/doc paths, and One variants. |
| src/unit_tests/rules/circuit_spinglass.rs | Update generic bounds to WeightElement. |
| src/unit_tests/models/graph/traveling_salesman.rs | Formatting + updated constructors/expectations after type-system changes. |
| src/unit_tests/graph_types.rs | Update subtype registration assertions to use One. |
| src/types.rs | Introduce WeightElement + One; remove NumericWeight/Weights; alias Unweighted = One. |
| src/traits.rs | Add SatisfactionProblem marker trait. |
| src/rules/variants.rs | New concrete-variant registrations for “orphan” nodes needed by natural edges (using One). |
| src/rules/travelingsalesman_ilp.rs | Formatting-only changes in ILP encoding implementation. |
| src/rules/spinglass_maxcut.rs | Make ReduceTo impls concrete (i32); update bounds via WeightElement. |
| src/rules/sat_ksat.rs | Limit SAT→K-SAT registration to K=3; update macro + comments. |
| src/rules/registry.rs | Update “base/unweighted” detection to One. |
| src/rules/mod.rs | Wire in new MIS→GridGraph rule module and variants module. |
| src/rules/minimumvertexcover_minimumsetcovering.rs | Make ReduceTo impl concrete (i32) and use WeightElement in bounds. |
| src/rules/minimumvertexcover_maximumindependentset.rs | Make ReduceTo impls concrete (i32) and use WeightElement in bounds. |
| src/rules/maximumsetpacking_qubo.rs | Make reduction concrete for MaximumSetPacking<f64> → QUBO<f64>; simplify implementation. |
| src/rules/maximummatching_maximumsetpacking.rs | Make ReduceTo impl concrete (SimpleGraph, i32); update bounds. |
| src/rules/maximumindependentset_maximumsetpacking.rs | Make ReduceTo impls concrete (i32); update bounds. |
| src/rules/maximumindependentset_gridgraph.rs | New MIS(Simple/UnitDisk)→MIS(GridGraph) reduction using KSG mapping. |
| src/rules/graph.rs | Reduction graph JSON: edges reference node indices; categories/docs derived from schema module_path; API uses Problem::NAME. |
| src/rules/coloring_ilp.rs | Make ReduceTo concrete for KColoring<K, SimpleGraph> → ILP. |
| src/registry/schema.rs | Add module_path field to ProblemSchemaEntry. |
| src/models/specialized/paintshop.rs | Add schema module_path; variant() now returns empty vec. |
| src/models/specialized/factoring.rs | Add schema module_path; variant() now returns empty vec. |
| src/models/specialized/circuit.rs | Add schema module_path; implement SatisfactionProblem; variant() now empty. |
| src/models/specialized/bmf.rs | Add schema module_path; variant() now returns empty vec. |
| src/models/specialized/biclique_cover.rs | Add schema module_path; variant() now returns empty vec. |
| src/models/set/minimum_set_covering.rs | Migrate to WeightElement and W::Sum metrics; variant() emits only weight key; add schema module_path. |
| src/models/set/maximum_set_packing.rs | Migrate to WeightElement and W::Sum metrics; variant() emits only weight key; add schema module_path. |
| src/models/satisfiability/sat.rs | Add schema module_path; implement SatisfactionProblem; variant() now empty. |
| src/models/satisfiability/ksat.rs | Add schema module_path; implement SatisfactionProblem; variant() now only k. |
| src/models/optimization/spin_glass.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/optimization/qubo.rs | Migrate to WeightElement and W::Sum metrics; variant() emits only weight key; add schema module_path. |
| src/models/optimization/ilp.rs | Add schema module_path; variant() now empty. |
| src/models/mod.rs | Minor reordering of re-exports. |
| src/models/graph/traveling_salesman.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/mod.rs | Reorder module declarations/exports (no functional change). |
| src/models/graph/minimum_vertex_cover.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/minimum_dominating_set.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/maximum_matching.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/maximum_independent_set.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/maximum_clique.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/maximal_is.rs | Migrate to WeightElement and W::Sum metrics; variant() uses G::NAME; add schema module_path. |
| src/models/graph/max_cut.rs | Migrate to WeightElement and W::Sum metrics; update cut_size return type; add schema module_path. |
| src/models/graph/kcoloring.rs | Add schema module_path; implement SatisfactionProblem; variant() uses G::NAME. |
| src/lib.rs | Update exports/prelude for One/WeightElement and SatisfactionProblem; remove old types. |
| src/graph_types.rs | Update declared weight subtype chain to One → i32 → f64. |
| src/export.rs | Update variant metadata doc comment to reference One. |
| problemreductions-macros/src/lib.rs | Enforce concrete ReduceTo impls (no type generics); remove fallback override attributes. |
| examples/reduction_maximumsetpacking_to_qubo.rs | Switch example to MaximumSetPacking<f64> for QUBO reduction. |
| examples/export_petersen_mapping.rs | Update output paths to docs/paper/static/. |
| docs/src/reductions/reduction_graph.json | Regenerated reduction graph export using node indices + new variant/category scheme. |
| docs/src/introduction.md | Update JS visualization to consume indexed edges and improved labels/tooltips. |
| docs/src/design.md | Rename doc heading, add variant-system explanation and examples. |
| docs/src/SUMMARY.md | Update nav entry from Architecture→Design. |
| docs/plans/2026-02-14-type-system-cleanup-impl.md | Add implementation plan doc for this refactor. |
| docs/plans/2026-02-14-type-system-cleanup-design.md | Add design doc for the refactor. |
| docs/paper/static/petersen_source.json | Add paper static JSON output (new path). |
| docs/paper/reductions.typ | Update Typst to follow indexed-edge JSON and new static paths. |
| .claude/skills/issue-to-pr.md | Tighten issue template guidance for reduction examples. |
Comments suppressed due to low confidence (2)
docs/src/design.md:106
- The variant examples/documentation still refer to
Unweighted(e.g.,MaximumIndependentSet<UnitDiskGraph, Unweighted>and("weight", "Unweighted")), but the codebase now uses the unit-weight typeOneand variant strings like "One". Update these examples/text to useOne(and adjust the note about omitting default fields accordingly) to avoid misleading readers.
docs/src/design.md:143 - This weight-hierarchy section documents
Unweighted → i32 → f64and showsdeclare_weight_subtype!("Unweighted" => "i32"), but the implementation was renamed toOne(seesrc/graph_types.rs). Please update the diagram/snippet (and any later references likeUnweighted ≤ i32) toOne → i32 → f64so it matches the current variant metadata.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[reduction(overhead = { | ||
| ReductionOverhead::new(vec![ | ||
| ("num_clauses", poly!(num_clauses) + poly!(num_literals)), | ||
| ("num_vars", poly!(num_vars) + poly!(num_literals)), | ||
| ]) | ||
| })] | ||
| ReductionOverhead::new(vec![ | ||
| ("num_clauses", poly!(num_clauses) + poly!(num_literals)), | ||
| ("num_vars", poly!(num_vars) + poly!(num_literals)), | ||
| ]) | ||
| })] |
There was a problem hiding this comment.
The #[reduction(overhead = { ... })] attribute inside impl_sat_to_ksat! has unusually deep indentation that doesn't match the surrounding style and is likely to fail fmt-check if the repo enforces rustfmt. Reformat this attribute (or run cargo fmt) so the macro expansion stays consistent with other reduction attributes in the crate.
src/rules/variants.rs
Outdated
| inventory::submit! { ConcreteVariantEntry { name: "MaximumIndependentSet", variant_fn: || vec![("graph", "SimpleGraph"), ("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "MinimumVertexCover", variant_fn: || vec![("graph", "SimpleGraph"), ("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "MaxCut", variant_fn: || vec![("graph", "SimpleGraph"), ("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "SpinGlass", variant_fn: || vec![("graph", "SimpleGraph"), ("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "MaximumMatching", variant_fn: || vec![("graph", "SimpleGraph"), ("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "MaximumSetPacking", variant_fn: || vec![("weight", "One")] } } | ||
| inventory::submit! { ConcreteVariantEntry { name: "MinimumSetCovering", variant_fn: || vec![("weight", "One")] } } |
There was a problem hiding this comment.
These inventory::submit! registrations are all written as single very-long lines. If the repo enforces rustfmt via make fmt-check, this file is likely to be reformatted (and may currently fail formatting checks). Consider formatting each submission as a multi-line block for consistency and to keep diffs stable.
docs/src/introduction.md
Outdated
| } | ||
|
|
||
| // Default values per variant key — omitted in concise labels | ||
| var variantDefaults = { graph: 'SimpleGraph', weight: 'Unweighted' }; |
There was a problem hiding this comment.
variantDefaults.weight is set to "Unweighted", but the reduction graph JSON and Problem::variant() now use "One" for unit-weight variants. This will cause base-variant detection (isBaseVariant) and label suppression to fail (e.g., default nodes will be treated as non-base and show "One" in labels). Update the default weight string to "One" (or accept both during a transition).
| var variantDefaults = { graph: 'SimpleGraph', weight: 'Unweighted' }; | |
| var variantDefaults = { graph: 'SimpleGraph', weight: 'One' }; |
Introduces a Triangular lattice graph type (subtype of UnitDiskGraph) and a reduction from MIS<SimpleGraph> to MIS<Triangular> using the weighted triangular unit disk mapping, with O(n²) overhead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix variantDefaults.weight from "Unweighted" to "One" in docs - Update design.md weight hierarchy docs to use "One" instead of "Unweighted" - Format variants.rs inventory::submit! as multi-line blocks - Apply rustfmt to sat_ksat.rs macro and other files - Add UnitDiskGraph → GridGraph reduction test for coverage - Add Triangular Graph trait method tests for coverage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove ConcreteVariantEntry and variants.rs — reduction graph nodes now come exclusively from explicit #[reduction] registrations. Natural edges between same-name variants are still inferred from the subtype partial order on graph/weight types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Wrap generic types in backticks in maximumsetpacking_qubo.rs doc comment - Update CLAUDE.md: Unweighted→One, add Triangular/WeightElement, fix variant ID docs Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add Triangular to graph types table and hierarchy diagram - Update variant discovery description to reflect automatic node inference Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduce GraphCast trait, ReductionAutoCast struct, and impl_natural_reduction! macro for declarative graph subtype relaxation reductions. First usage: MIS<Triangular> → MIS<SimpleGraph>. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 77 out of 81 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
module_pathinstead of manual annotation, use node indices in reduction graph JSON, drastically reducingreduction_graph.jsonsizeWeightElementtrait andOnetype replacingUnweighted/Weights/NumericWeight, addSatisfactionProblemmarker trait, make#[reduction]macro ReduceTo impls concrete instead of using hardcoded fallbacksTriangularlattice graph (subtype ofUnitDiskGraph) with a#[reduction]-based MIS reduction fromSimpleGraphusing the weighted triangular unit disk mapping (O(n²) overhead)docs/paper/static/subdirectoryTest plan
make testpassesmake clippypassesmake fmt-checkpasses🤖 Generated with Claude Code