Fix #52: TravelingSalesman to ILP reduction#60
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #60 +/- ##
==========================================
+ Coverage 96.95% 97.01% +0.05%
==========================================
Files 176 178 +2
Lines 25386 25540 +154
==========================================
+ Hits 24614 24778 +164
+ Misses 772 762 -10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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>
There was a problem hiding this comment.
Pull request overview
This pull request implements a reduction from TravelingSalesman to ILP (Integer Linear Programming) using position-based binary variables with McCormick linearization to handle the quadratic objective function. The PR also includes a refactoring effort to migrate several existing ILP reductions to use the #[reduction] macro attribute instead of manual inventory submissions.
Changes:
- Implements TravelingSalesman → ILP reduction with comprehensive unit tests and example program
- Refactors existing ILP reductions (MinimumVertexCover, MinimumSetCovering, MinimumDominatingSet, MaximumSetPacking, MaximumMatching, MaximumIndependentSet, MaximumClique, Factoring, KColoring, SAT → KSAT) to use
#[reduction]macro - Updates documentation including paper (Typst), reduction graph JSON, and introduction/getting-started guides
Reviewed changes
Copilot reviewed 23 out of 24 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/rules/travelingsalesman_ilp.rs |
Core reduction implementation using position-based encoding with n² main variables and 2mn auxiliary variables for McCormick linearization |
src/unit_tests/rules/travelingsalesman_ilp.rs |
Comprehensive test suite covering valid ILP generation, closed-loop correctness, weighted/unweighted instances, infeasible cases, and solution extraction |
examples/reduction_travelingsalesman_to_ilp.rs |
Example program demonstrating K4 weighted instance reduction with brute force validation |
docs/paper/reductions.typ |
Mathematical documentation of the reduction with detailed construction and solution extraction |
docs/src/reductions/reduction_graph.json |
Updated reduction graph with TravelingSalesman nodes and edges |
src/rules/mod.rs |
Module registration for new reduction |
tests/suites/examples.rs |
Example test registration |
src/rules/*_ilp.rs |
Consistent refactoring to use #[reduction] macro |
src/rules/sat_ksat.rs |
Refactoring to use #[reduction] macro within macro-generated code |
docs/src/introduction.md |
Updated library name formatting |
docs/src/getting-started.md |
Updated library name formatting |
Makefile, .gitignore, .claude/* |
Development workflow and configuration updates |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/rules/travelingsalesman_ilp.rs
Outdated
| // num_constraints = 2n + n^2*(n-density) + 6mn ≈ n^3 + 6mn | ||
| ("num_constraints", Polynomial::var_pow("num_vertices", 3) + Polynomial { | ||
| terms: vec![Monomial { | ||
| coefficient: 6.0, | ||
| variables: vec![("num_vertices", 1), ("num_edges", 1)], | ||
| }] |
There was a problem hiding this comment.
The overhead polynomial for num_constraints is documented as an approximation ("≈ n^3 + 6mn"), but the actual formula should be more precise. According to the plan documentation, the exact count is 2n + n(n(n-1) - 2m) + 6mn = n^3 - n^2 + 2n + 4mn. The approximation omits the -n^2 + 2n + 4mn terms (keeping only the dominant n^3 term plus 6mn instead of 4mn). While polynomial approximations are sometimes acceptable for asymptotic analysis, for a reduction overhead specification it would be more accurate to either: (1) use the exact formula, or (2) clearly document why this specific approximation was chosen and what terms were dropped.
| // num_constraints = 2n + n^2*(n-density) + 6mn ≈ n^3 + 6mn | |
| ("num_constraints", Polynomial::var_pow("num_vertices", 3) + Polynomial { | |
| terms: vec![Monomial { | |
| coefficient: 6.0, | |
| variables: vec![("num_vertices", 1), ("num_edges", 1)], | |
| }] | |
| // num_constraints = 2n + n(n(n-1) - 2m) + 6mn = n^3 - n^2 + 2n + 4mn | |
| ("num_constraints", Polynomial::var_pow("num_vertices", 3) + Polynomial { | |
| terms: vec![ | |
| Monomial { | |
| coefficient: -1.0, | |
| variables: vec![("num_vertices", 2)], | |
| }, | |
| Monomial { | |
| coefficient: 2.0, | |
| variables: vec![("num_vertices", 1)], | |
| }, | |
| Monomial { | |
| coefficient: 4.0, | |
| variables: vec![("num_vertices", 1), ("num_edges", 1)], | |
| }, | |
| ] |
docs/src/introduction.md
Outdated
| **problemreductions** provides implementations of various computational hard problems and reduction rules between them. It is designed for algorithm research, education, and industry applications. | ||
|
|
||
| For theoretical background and correctness proofs, see the [PDF manual](https://codingthrust.github.io/problem-reductions/reductions.pdf). | ||
| **problem-reductions** a rust library that provides implementations of various computational hard problems and reduction rules between them. It is designed for algorithm research, education, and industry applications. |
There was a problem hiding this comment.
Missing article "is" before "a rust library". The sentence should read: "problem-reductions is a rust library that provides..." instead of "problem-reductions a rust library that provides..."
| **problem-reductions** a rust library that provides implementations of various computational hard problems and reduction rules between them. It is designed for algorithm research, education, and industry applications. | |
| **problem-reductions** is a rust library that provides implementations of various computational hard problems and reduction rules between them. It is designed for algorithm research, education, and industry applications. |
| { | ||
| "name": "TravelingSalesman", | ||
| "variant": {}, | ||
| "category": "other", | ||
| "doc_path": "models/specialized/struct.TravelingSalesman.html" | ||
| }, | ||
| { | ||
| "name": "TravelingSalesman", | ||
| "variant": { | ||
| "graph": "SimpleGraph", | ||
| "weight": "i32" | ||
| }, | ||
| "category": "other", | ||
| "doc_path": "models/specialized/struct.TravelingSalesman.html" | ||
| } |
There was a problem hiding this comment.
The TravelingSalesman problem is incorrectly categorized. It should have category "graph" (since it's in src/models/graph/traveling_salesman.rs) instead of "other", and the doc_path should be "models/graph/struct.TravelingSalesman.html" instead of "models/specialized/struct.TravelingSalesman.html". This appears in both TravelingSalesman node entries (lines 395-409) in the nodes array.
- Use exact constraint count formula (n³ - n² + 2n + 4mn) instead of approximation - Fix missing "is" in introduction.md - Categorize TravelingSalesman as "graph" with correct doc_path Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Closes #52