Fix #90: Add ClosestVectorProblem model#92
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #92 +/- ##
==========================================
- Coverage 96.90% 96.89% -0.01%
==========================================
Files 198 200 +2
Lines 26908 27099 +191
==========================================
+ Hits 26074 26258 +184
- Misses 834 841 +7 ☔ 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 adds a comprehensive implementation plan for the ClosestVectorProblem (CVP) optimization model. CVP is a fundamental lattice problem where, given a lattice basis B and target vector t, the goal is to find integer coefficients x that minimize the Euclidean distance ‖Bx - t‖₂.
The plan follows the established add-model skill pattern with 7 tasks covering all aspects of model integration: struct definition, trait implementations, module/CLI registration, comprehensive testing, and paper documentation. The implementation parameterizes CVP by element type T (supporting both i32 for integer lattices and f64 for real lattices) and reuses VarBounds from ILP for specifying enumeration bounds on integer variables.
Changes:
- Adds detailed implementation plan with step-by-step TDD approach for CVP model
- Includes struct design with basis matrix (column vectors), target vector, and variable bounds
- Provides comprehensive test suite covering evaluation, solvers, serialization, and edge cases
- Adds paper documentation with mathematical definition matching issue #90 specification
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Add match arm in `serialize_any_problem()` (after the `"ILP"` arm): | ||
| ```rust | ||
| "ClosestVectorProblem" => try_ser::<ClosestVectorProblem<i32>>(data), |
There was a problem hiding this comment.
Similarly, the serialize_any_problem function should support both i32 and f64 variants. Following the SpinGlass pattern (lines 281-284), this should check the variant map:
"ClosestVectorProblem" => match variant.get("weight").map(|s| s.as_str()) {
Some("f64") => try_ser::<ClosestVectorProblem<f64>>(any),
_ => try_ser::<ClosestVectorProblem<i32>>(any),
},
| Add match arm in `serialize_any_problem()` (after the `"ILP"` arm): | |
| ```rust | |
| "ClosestVectorProblem" => try_ser::<ClosestVectorProblem<i32>>(data), | |
| Add match arm in `serialize_any_problem()` (after the `"ILP"` arm), following the SpinGlass pattern and using the `variant` map to select the weight type: | |
| ```rust | |
| "ClosestVectorProblem" => match variant.get("weight").map(|s| s.as_str()) { | |
| Some("f64") => try_ser::<ClosestVectorProblem<f64>>(data), | |
| _ => try_ser::<ClosestVectorProblem<i32>>(data), | |
| }, |
| Add to `src/models/optimization/closest_vector_problem.rs`, requiring `T: Into<f64> + Clone`: | ||
|
|
||
| ```rust | ||
| impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> Problem for ClosestVectorProblem<T> { |
There was a problem hiding this comment.
The trait bounds should include crate::variant::VariantParam for type T. This is required for the variant system to work properly with the variant_params macro. Following the pattern from SpinGlass (line 200-202 in src/models/optimization/spin_glass.rs), the bounds should be:
T: Clone + Into<f64> + crate::variant::VariantParam + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static
| impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> Problem for ClosestVectorProblem<T> { | |
| impl<T: Clone + Into<f64> + crate::variant::VariantParam + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> Problem for ClosestVectorProblem<T> { |
| } | ||
| } | ||
|
|
||
| impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> OptimizationProblem for ClosestVectorProblem<T> { |
There was a problem hiding this comment.
The OptimizationProblem trait implementation should also include crate::variant::VariantParam in the trait bounds for type T, matching the Problem trait implementation and following the pattern from SpinGlass (line 237-238).
| impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> OptimizationProblem for ClosestVectorProblem<T> { | |
| impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + crate::variant::VariantParam + 'static> OptimizationProblem for ClosestVectorProblem<T> { |
| } | ||
|
|
||
| fn variant() -> Vec<(&'static str, &'static str)> { | ||
| crate::variant_params![] |
There was a problem hiding this comment.
The variant function should declare the type parameter T since ClosestVectorProblem is parameterized by element type. Both i32 and f64 implement VariantParam with category "weight" (see src/types.rs:248-249), so this should be crate::variant_params![T] instead of crate::variant_params![]. Without this, the variant system won't properly track the i32 vs f64 distinction, which could cause issues with serialization and CLI dispatch.
| crate::variant_params![] | |
| crate::variant_params![T] |
|
|
||
| ```rust | ||
| use super::*; | ||
| use crate::models::optimization::VarBounds; |
There was a problem hiding this comment.
This import is redundant. The test file already uses super::* at line 23, which imports all public items from the parent module (optimization), including VarBounds. This explicit import can be removed.
| use crate::models::optimization::VarBounds; |
| "ClosestVectorProblem" => deser_opt::<ClosestVectorProblem<i32>>(data), | ||
| ``` |
There was a problem hiding this comment.
The CLI dispatch should support both i32 and f64 variants of ClosestVectorProblem, as specified in issue #90. Currently only i32 is registered. Following the pattern from SpinGlass (lines 230-233 in problemreductions-cli/src/dispatch.rs), this should check the variant map and dispatch to the appropriate type. The match arm should be:
"ClosestVectorProblem" => match variant.get("weight").map(|s| s.as_str()) {
Some("f64") => deser_opt::<ClosestVectorProblem<f64>>(data),
_ => deser_opt::<ClosestVectorProblem<i32>>(data),
},
Changes from original plan: - Remove problem_size_names/values (removed from Problem trait) - Add declare_variants! with complexity metadata for i32/f64 variants - Add VariantParam trait bound on T parameter - Use variant_params![T] instead of variant_params![] - CLI dispatch supports both i32 and f64 via variant map (SpinGlass pattern) - Remove redundant VarBounds import in tests (super::* suffices) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add CVP as a new optimization model parameterized by element type T (i32/f64). Implements Problem and OptimizationProblem traits, registers in CLI dispatch with "CVP" alias, and adds problem definition to paper. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn evaluate(&self, config: &[usize]) -> SolutionSize<f64> { | ||
| let values = self.config_to_values(config); | ||
| let m = self.ambient_dimension(); | ||
| let mut diff = vec![0.0f64; m]; | ||
| for (i, &x_i) in values.iter().enumerate() { | ||
| for (j, b_ji) in self.basis[i].iter().enumerate() { | ||
| diff[j] += x_i as f64 * b_ji.clone().into(); | ||
| } | ||
| } |
There was a problem hiding this comment.
evaluate can panic or produce incorrect results for malformed configs. If config.len() > num_basis_vectors(), indexing self.basis[i] will go out of bounds; if config.len() < num_basis_vectors(), the remaining variables are silently ignored. Consider returning SolutionSize::Invalid when config.len() != self.num_basis_vectors() and (optionally) when any decoded value violates bounds[i] / config[i] >= dims[i], similar to how other models treat invalid configurations.
docs/paper/reductions.typ
Outdated
| #problem-def("ClosestVectorProblem")[ | ||
| Given a lattice basis $bold(B) in RR^(m times n)$ (columns $bold(b)_1, dots, bold(b)_n in RR^m$ spanning lattice $cal(L)(bold(B)) = {bold(B) bold(x) : bold(x) in ZZ^n}$) and target $bold(t) in RR^m$, find $bold(x) in ZZ^n$ minimizing $norm(bold(B) bold(x) - bold(t))_2$. | ||
| ][ | ||
| The Closest Vector Problem is a fundamental lattice problem, proven NP-hard by van Emde Boas @vanemde1981. CVP plays a central role in lattice-based cryptography and the geometry of numbers. Kannan's algorithm @kannan1987 solves CVP in $O^*(n^n)$ time using the Hermite normal form, later improved to $O^*(2^n)$ via the randomized sieve of Micciancio and Voulgaris @micciancio2010. CVP is closely related to the Shortest Vector Problem (SVP) and integer programming: Lenstra's algorithm for fixed-dimensional ILP @lenstra1983 proceeds via CVP in the dual lattice. |
There was a problem hiding this comment.
The new CVP definition introduces citation keys @vanemde1981, @kannan1987, and @micciancio2010, but these keys do not appear in docs/paper/references.bib. This will likely result in unresolved citations (and may fail the paper build, depending on CI settings). Add the missing BibTeX entries or change the citation keys to ones that already exist in the bibliography.
| The Closest Vector Problem is a fundamental lattice problem, proven NP-hard by van Emde Boas @vanemde1981. CVP plays a central role in lattice-based cryptography and the geometry of numbers. Kannan's algorithm @kannan1987 solves CVP in $O^*(n^n)$ time using the Hermite normal form, later improved to $O^*(2^n)$ via the randomized sieve of Micciancio and Voulgaris @micciancio2010. CVP is closely related to the Shortest Vector Problem (SVP) and integer programming: Lenstra's algorithm for fixed-dimensional ILP @lenstra1983 proceeds via CVP in the dual lattice. | |
| The Closest Vector Problem is a fundamental lattice problem, proven NP-hard by van Emde Boas (1981). CVP plays a central role in lattice-based cryptography and the geometry of numbers. Kannan's algorithm (1987) solves CVP in $O^*(n^n)$ time using the Hermite normal form, later improved to $O^*(2^n)$ via the randomized sieve of Micciancio and Voulgaris (2010). CVP is closely related to the Shortest Vector Problem (SVP) and integer programming: Lenstra's algorithm for fixed-dimensional ILP @lenstra1983 proceeds via CVP in the dual lattice. |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix factual errors: Kannan runs in n^O(n) (not n^n), Micciancio-Voulgaris is deterministic O*(4^n) Voronoi cell (not randomized sieve O*(2^n)) - Add Aggarwal-Dadush-Stephens-Davidowitz 2015 citation for the O*(2^n) bound - Add BibTeX entries: vanemde1981, kannan1987, micciancio2010, aggarwal2015 - Add lattice visualization figure with basis vectors, target, and closest point - Add evaluation walkthrough showing distance computation - Fix Makefile: mkdir -p tests/julia before rust-export - Regenerate problem_schemas.json and reduction_graph.json Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
Closes #90
ClosestVectorProblem<T>optimization model: given lattice basis B and target t, minimize ‖Bx - t‖₂ over integer xdeclare_variants!registrationTest plan
make test— all tests pass (1525+ unit tests, 44 doc tests)make clippy— no warnings🤖 Generated with Claude Code