Repository navigation
Derive reduction parameter contracts and bound ILP encodings - #1174
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1174 +/- ##
==========================================
+ Coverage 96.73% 96.88% +0.15%
==========================================
Files 1069 1095 +26
Lines 138549 144504 +5955
==========================================
+ Hits 134023 140006 +5983
+ Misses 4526 4498 -28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Agentic Review ReportReviewed head Verdict: changes requested. The infrastructure is sound, but six formulas declared Structural
Quality
Feature test (
|
| Command | Result |
|---|---|
pred show QUBO |
lists num_quadratic_terms; per-field = / <= / unavailable rendered correctly |
pred path HamiltonianPath ILP |
three exact formulas using num_consecutive_positions |
pred path KSatisfiability/K2 QUBO |
overall num_vars =, num_quadratic_terms <= (mixed relation composes) |
pred path QUBO ILP inst.json on the canonical example |
measured 5 / 6 / 14, matches n + m, 3m, 7m |
| Counterexamples below | all reproduced |
Findings
All reproduced with pred inspect + pred path <S> <T> instance.json on the PR head.
Critical — formulas declared exact that are false on valid instances
| # | Location | Instance | Declared | Measured |
|---|---|---|---|---|
| 1 | src/rules/coloring_qubo.rs:185 num_quadratic_terms (new in this PR) |
KColoring, 2 vertices, edges [(0,1),(0,1)], 3 colors |
12 | 9 |
| 2 | src/rules/travelingsalesman_ilp.rs:69-72 num_constraints |
TSP, 2 vertices, edges [(0,1),(0,1)] |
24 | 28 |
| 3 | src/rules/ruralpostman_ilp.rs:44-47 num_vars, num_constraints |
2 vertices, 1 edge, required_edges = [] |
6, 16 | 0, 0 |
| 4 | src/rules/minimummaximalmatching_ilp.rs:52-55 num_constraints |
3 vertices, edges [(0,1)] (isolated vertex) |
4 | 3 |
| 5 | src/rules/minimumedgecostflow_ilp.rs:57-60 num_constraints |
3 vertices, arcs [(0,1)], source 0, sink 1 |
4 | 3 |
| 6 | src/rules/minimumfaultdetectiontestset_ilp.rs:46-54 num_constraints and the new num_nonzeros bound |
2 vertices, arcs [(0,1),(1,0)], inputs [0], outputs [0] |
0, ≤ 0 | 1, 1 |
Notes:
- Implement remaining reduction rules #2 undercounts, so relabelling it
upper_boundis not enough. The non-edge block useshas_edge(distinct pairs) while the McCormick block iterates all edges. Either dedupe edges in the rule or reject parallel edges in the model. - Expand ILP solver support to more problem types #6 with inputs
[0,0], outputs[1]evaluates to-1, soevaluate()returnsNegativeResult. Root cause: the model does not require inputs and outputs to be distinct and disjoint. - Roadmap #1 is introduced here. Implement remaining reduction rules #2–Expand ILP solver support to more problem types #6 predate the PR but are re-declared in its
exactblocks, and the PR's claim is that every declared formula is verified. - Suggested fixes: Roadmap #1, feat: Feature parity with ProblemReductions.jl #4, Add HiGHS ILP solver support #5 move to
upper_bound; Coding Rules - For AI agents to follow #3 move toupper_boundor drop the early return at:61-67; Expand ILP solver support to more problem types #6 validate the model or bound bynum_vertices. maximummatching_ilp.rsgot exactly this treatment in this PR (isolated vertices, testparameter_predictions_account_for_isolated_vertices_and_loops); feat: Feature parity with ProblemReductions.jl #4 is the same construction.
Important
- New variable bounds turn infeasible instances into reduction errors.
src/rules/pathconstrainednetworkflow_ilp.rs:74-79andsrc/rules/undirectedflowlowerbounds_ilp.rs:176-183buildIntegerVariable::new(Some(0), Some(capacity)). Neither model rejects negative capacities, so a capacity of-1now fails withinteger variable lower bound exceeds its upper bound; before the PR it produced an infeasible ILP. Under the chain contract a failed reduction is an error, not proof of infeasibility. Fix at the root: reject negative capacities in both constructors. - Weak verification. Add adversarial instances to the shared test for every
exactfield: isolated vertices, parallel edges and self-loops (SimpleGraph::newaccepts both), empty required/terminal sets, and size 0/1. Findings 1–6 are the ones found by hand; the test design will miss the next one. - Undocumented breaking changes. CLI contract JSON moves
relationfrom the contract level into eachfields[]entry (problemreductions-cli/src/commands/graph.rs:667-677).ParameterContractError::{EmptyTransform, MissingRelation}andParameterTransform::relation()(no argument) are removed from the public API.path_parameter_transformsno longer returnsPathParameterError::Unavailable. None is in the PR description. QUBO<f64> → SpinGlassbehaviour change.src/rules/spinglass_qubo.rs:63-77drops the1e-10threshold so thatnum_interactions = num_quadratic_termsis exact. Couplings that were filtered are now kept. Reasonable, but it is a semantic change and should be stated.- Merge conflicts with
mainmust be resolved. Theadd-ruleskill edit should move tohow-to-code/how-to-verify, and the.claude/CLAUDE.mdedits need rebasing onto the Replace board-driven pipelines with agent-invoked how-to guides #1173 rewrite.
Minor
SpinGlass → QUBOboundnum_spins * (num_spins - 1) / 2discards sparsity;num_quadratic_terms <= num_interactionsalways holds and composes better intoQUBO → ILP.- Most new
num_nonzerosbounds are the genericnum_vars * num_constraintsproduct. Valid (rows are normalized), but tight linear forms are available, e.g.maximummatching_ilpis exactly2 * num_edges,maximalis_ilp4 * num_edges + num_vertices,minimumdominatingset_ilp2 * num_edges + num_vertices. - Fields declared
upper_boundthat are exact:minimumcapacitatedspanningtree_ilp(num_vars,num_constraints),lengthboundeddisjointpaths_ilpandmaximumsetpacking_ilp(num_vars),hamiltoniancircuit_hamiltonianpath(num_vertices,num_consecutive_positions),paintshop_ilp(num_vars). knapsack_ilp.rs:43reads"num_items * 1".- Stray blank lines left where
relation:was removed:src/models/decision.rs:82,127,problemreductions-cli/src/test_support.rs:402,433,subsetsum_integerknapsack.rs:35. - Bounded
ILP<i64>rules keep their explicitx <= boundrows alongside the new variable bounds. Correct, and the exactnum_constraintsformulas match the code as written; removing the rows later needs the formulas updated in the same change.
Pre-existing, not introduced here
SpinGlass → QUBO(src/rules/spinglass_qubo.rs:145-151,:186-201) writesmatrix[i][j]without ordering the endpoints, andQUBO::from_matrixreads only the upper triangle. A SpinGlass with edge(1,0), coupling 1 reduces to QUBO entries[(0,0,-2),(1,1,-2)]with no quadratic term, so the coupling is lost. Worth a separate issue.
Verified correct
Hand-counted and confirmed exact, including size-0/1 cases: hamiltonianpath_ilp (all three), quadraticassignment_ilp, qubo_ilp, qubo_casts, bmf_ilp, closeststring_ilp, consistencyofdatabasefrequencytables_ilp, exactcoverby3sets_ilp, expectedretrievalcost_ilp, feasibleregisterassignment_ilp, graphpartitioning_qubo, integerknapsack_ilp, longestcommonsubsequence_ilp, maximumcontactmapoverlap_ilp, maximumlikelihoodranking_ilp, minimummatrixcover_ilp, registersufficiency_ilp, sumofsquarespartition_ilp, threedimensionalmatching_ilp, and the four non-ILP rules. Apart from finding 7, the new with_variables bounds are implied by existing rows, cut no feasible or optimal point, add no panics or unchecked casts, and leave extract_solution unchanged.
Correct exactness claims, count sparse coefficients by construction block, reject negative flow capacities, and verify metadata using existing behavior inputs and real executors.
Derive upper bounds from conditional even padding and the set-packing endpoint universe. Preserve the exact edge-to-set count and verify the registered promises in existing parity and isolated-vertex tests.
Replace oversized formulations with compact constructions for partition, register, matrix, graph, and ordering problems. Add direct bounded ILP pipelines and reuse exact customized subset-sum and clique-cover solvers. Preserve signed weights and costs, avoid artificial partition-bound overflow, and align overhead contracts, rule targets, tests, and proofs with the resulting constructions. Validated with make check, make paper, independent reduction audits, and CLI solver and extraction round trips.
GiggleLiu
left a comment
There was a problem hiding this comment.
Reviewed the parameter contracts, bounded ILP encodings and solver changes, including independent review of the follow-up fixes. Two preprocessing regressions were found and fixed: unbounded work before small quadratic witnesses, and exponential subset generation for ensemble instances. The fixes also cover the broader cutoff and feasible-large-set cases, with complete fallback searches and compact ILP fallback. The current-CI Clippy warning is fixed. No blocking findings remain; policy audit PASS.
Validation on 8377fe5: make check and make paper pass; all seven CI workflow jobs pass, including tests, coverage and platform builds. The no-mistakes runner was unavailable because its configured Claude account is restricted; independent review and repository checks were performed directly.
…1190) * Make derivable reduction parameters exact * Check every field in exact reduction transforms * Verify exact reduction parameters on randomized instances * Check exact and upper-bound reduction parameters * Document and simplify parameter formula validation * Improve parameter prediction contracts and bound integer ILP reductions * Derive reduction parameter bounds from constructed targets Correct exactness claims, count sparse coefficients by construction block, reject negative flow capacities, and verify metadata using existing behavior inputs and real executors. * Consolidate bounded ILP and parameter prediction stack Squash the combined changes from PRs #1180, #1182, #1183, #1184, and #1185 into #1174. Preserve the complete stack-tip tree so subsequent corrections can be maintained on one branch. * Correct ILP parameter bounds and simplify reduction code * Calibrate parity and universe-size reduction bounds Derive upper bounds from conditional even padding and the set-packing endpoint universe. Preserve the exact edge-to-set count and verify the registered promises in existing parity and isolated-vertex tests. * Remove invalid reduction catalog edges * Add direct binary ILP pipelines for exact-one SAT and graph kernels * Preserve scheduling semantics with compact ILP constructions * Simplify scheduling solution extraction * Tighten ILP nonzero bounds using construction counts * Compact exact reductions and register missing solver pipelines Replace oversized formulations with compact constructions for partition, register, matrix, graph, and ordering problems. Add direct bounded ILP pipelines and reuse exact customized subset-sum and clique-cover solvers. Preserve signed weights and costs, avoid artificial partition-bound overflow, and align overhead contracts, rule targets, tests, and proofs with the resulting constructions. Validated with make check, make paper, independent reduction audits, and CLI solver and extraction round trips. * Remove redundant reduction parameters and derive bounds from model inputs * Fix exact verification bottlenecks in reduction targets * Simplify reduction results and exact solver bookkeeping * Tighten construction overhead bounds and expose required source statistics * Use lattice geometry and cached rectangle incidence to tighten remaining bounds * Sum individual coefficient width bounds for lattice encodings * Reuse constructed parameters in overhead count tests * fix: bound solver precomputation by small witness budgets * fix: remove needless borrow flagged by current CI clippy * fix: cap solver preprocessing and handle large union chains --------- Co-authored-by: GiggleLiu <cacate0129@gmail.com>
Reduction metadata could claim exact target sizes when construction skips rows, merges coefficients, or returns an empty target. Integer-to-binary paths also advertised general ILP even though encoding requires explicit finite variable bounds. This PR derives parameter contracts from the concrete constructions and makes that encoding precondition explicit in the reduction graph.
ILP<V, C, B = General>and the bounded integer variant. Supply finite intervals in incoming reductions, target binary ILP directly where applicable, and restrict integer-to-binary encoding to bounded ILP. Update solver dispatch, CLI references, and documentation.h, bounded integer ILP uses at mostn(h+1)binary variables; binary ILP withnvariables andmconstraints produces at mostU = n + m(n+h)QUBO variables andU²quadratic terms.For example, BinPacking with sizes
[1,1]and capacity1predicts at most 34 QUBO variables along its ILP route; the actual construction has 8, and solving and recovering gives source optimumMin(2).Validation: the complete stack passed
make checkandmake paperbefore consolidation. The squash commit has exactly the same Git tree as the reviewed stack tip1f16a331;git diff --cached --checkpassed. Consolidation changes commit history and PR organization only. Existing behavior tests and temporary diagnostics supplement the construction-based derivations; no additional instance fixtures were introduced for the review.The loop/duplicate-edge parameter declarations in Coloring → ILP, MinimumCoveringByCliques → ILP, and MaximumEdgeWeightedKClique → ILP have been corrected. The previously reported SpinGlass → QUBO reversed-endpoint bug and scheduling input-domain gaps are outside these fixes. Fixed-width arithmetic and solver numerical limits still apply; not every symbolic field or composed path is predictable.
This is now the single PR for the work previously split across #1180, #1182, #1183, #1184, and #1185. Their combined changes were squash-merged into this branch as
8bdddd45; the separate PRs are superseded.Related: #1175, #1176.
Review follow-up through
8377fe5afixes two solver preprocessing regressions and their broader cases. Quadratic congruences and Diophantine solving always search a small positive-witness prefix before factoring. Factorization, prime-residue scans and CRT combination allocation share a work budget; exhaustion resumes complete witness enumeration instead of reporting infeasibility. EnsembleComputation rejects impossible union budgets before generating subsets, constructs an optimal chain for one distinct required set (including feasible 64-element sets), and bounds subset/partition enumeration before falling back to the existing compact ILP encoding for larger multi-set cases. Output allocation failures remain typed errors.Regression tests cover both sides of the former quadratic cutoff, feasible and infeasible fallback searches, zero/nonunit residues, large ensemble chains, duplicate requirements, compact ILP fallback and allocation errors. Independent review found no blocking correctness issues. The follow-up also removes a needless borrow flagged by CI's Rust 1.99 Clippy.
make check,make paper, and all seven CI workflow jobs pass on8377fe5a. The no-mistakes runner could not start its review because its configured Claude Code account is restricted; independent review and repository checks were run directly instead.