Fix #167: Add TravelingSalesman to QUBO reduction#605
Fix #167: Add TravelingSalesman to QUBO reduction#605GiggleLiu merged 7 commits intoCodingThrust:mainfrom
Conversation
Co-authored-by: Claude <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #605 +/- ##
==========================================
+ Coverage 96.61% 96.70% +0.09%
==========================================
Files 218 220 +2
Lines 29352 29474 +122
==========================================
+ Hits 28357 28503 +146
+ Misses 995 971 -24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Position-based encoding (Lucas 2014): n² binary variables x_{v,p}
with one-hot row/column constraints and distance objective.
Handles non-complete graphs by penalizing non-edge consecutive pairs.
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
Implementation SummaryChanges
Deviations from Plan
Open Questions
Co-authored with Claude (opus-4.6) |
There was a problem hiding this comment.
Pull request overview
Adds a new reduction from TravelingSalesman<SimpleGraph, i32> to QUBO<f64>, along with an executable example and tests, and wires the example into the integration test suite.
Changes:
- Implement
TravelingSalesman -> QUBOreduction using position-based (n²) binary encoding with penalty constraints. - Add unit tests validating closed-loop optimality and basic size expectations for the new reduction.
- Add a runnable example and generated docs JSON outputs; include the example in
tests/suites/examples.rs.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/suites/examples.rs | Adds the new TSP→QUBO example module and corresponding test invocation. |
| src/unit_tests/rules/travelingsalesman_qubo.rs | Introduces unit tests for correctness and sizing of the reduction. |
| src/rules/travelingsalesman_qubo.rs | Implements the new reduction and solution extraction logic. |
| src/rules/mod.rs | Registers the new reduction module for compilation/registration. |
| examples/reduction_travelingsalesman_to_qubo.rs | New example demonstrating reduction, solving, extraction, and export. |
| docs/paper/examples/travelingsalesman_to_qubo.json | Adds exported reduction structure JSON for the paper docs. |
| docs/paper/examples/travelingsalesman_to_qubo.result.json | Adds exported solution pairs JSON for the paper docs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Build edge weight map (both directions for undirected lookup) | ||
| let mut edge_weight_map: HashMap<(usize, usize), f64> = HashMap::new(); | ||
| for (u, v, w) in self.edges() { | ||
| let wf = w as f64; | ||
| edge_weight_map.insert((u, v), wf); | ||
| edge_weight_map.insert((v, u), wf); | ||
| } | ||
|
|
||
| // Build edge index map: canonical (min, max) → edge index | ||
| let mut edge_index: HashMap<(usize, usize), usize> = HashMap::new(); | ||
| for (idx, &(u, v)) in graph_edges.iter().enumerate() { | ||
| edge_index.insert((u.min(v), u.max(v)), idx); | ||
| } | ||
|
|
||
| // Penalty weight: must exceed any possible tour cost | ||
| let weight_sum: f64 = self.edges().iter().map(|(_, _, w)| (*w as f64).abs()).sum(); | ||
| let a = 1.0 + weight_sum; | ||
|
|
There was a problem hiding this comment.
reduce_to() calls self.edges() multiple times (once to build edge_weight_map, again to compute weight_sum). TravelingSalesman::edges() allocates a new Vec each call, so this adds avoidable allocations/cloning. Consider calling self.edges() once and reusing the collected edges/weights for both the map and the penalty computation (or otherwise compute weight_sum while building the map).
| pub struct ReductionTravelingSalesmanToQUBO { | ||
| target: QUBO<f64>, | ||
| num_vertices: usize, | ||
| edges: Vec<(usize, usize)>, | ||
| edge_index: HashMap<(usize, usize), usize>, | ||
| } |
There was a problem hiding this comment.
ReductionTravelingSalesmanToQUBO stores edges: Vec<(usize, usize)>, but it’s only used for self.edges.len() when sizing the extracted source config. This keeps an extra copy of the graph’s edge list alive for the lifetime of the reduction result. Consider storing just num_edges (or deriving the length from edge_index.len()) instead of storing the full edges vector.
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Claude <noreply@anthropic.com>
…all self.edges() once Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Review Pipeline Report
🤖 Generated by review-pipeline |
Summary
Fixes #167
Co-authored with Claude (opus-4.6)