Skip to content

Commit b720481

Browse files
GiggleLiuclaude
andcommitted
docs: update CVP plan for refactored variant system and Copilot review
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>
1 parent ec23ad8 commit b720481

1 file changed

Lines changed: 35 additions & 32 deletions

File tree

docs/plans/2026-02-22-closest-vector-problem.md

Lines changed: 35 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ Create `src/unit_tests/models/optimization/closest_vector_problem.rs`:
2121

2222
```rust
2323
use super::*;
24-
use crate::models::optimization::VarBounds;
2524
use crate::traits::{OptimizationProblem, Problem};
2625
use crate::types::{Direction, SolutionSize};
2726

@@ -161,7 +160,7 @@ git commit -m "feat: add ClosestVectorProblem struct and constructor"
161160

162161
---
163162

164-
### Task 2: Implement Problem and OptimizationProblem traits
163+
### Task 2: Implement Problem and OptimizationProblem traits with declare_variants!
165164

166165
**Files:**
167166
- Modify: `src/models/optimization/closest_vector_problem.rs`
@@ -216,10 +215,15 @@ Expected: FAIL (trait not implemented)
216215

217216
**Step 3: Implement the traits**
218217

219-
Add to `src/models/optimization/closest_vector_problem.rs`, requiring `T: Into<f64> + Clone`:
218+
Add to `src/models/optimization/closest_vector_problem.rs`.
219+
220+
Note: `T` must have `crate::variant::VariantParam` trait bound (category "weight") for the variant system to work. Follow the SpinGlass pattern.
220221

221222
```rust
222-
impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> Problem for ClosestVectorProblem<T> {
223+
impl<T> Problem for ClosestVectorProblem<T>
224+
where
225+
T: Clone + Into<f64> + crate::variant::VariantParam + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static,
226+
{
223227
const NAME: &'static str = "ClosestVectorProblem";
224228
type Metric = SolutionSize<f64>;
225229

@@ -236,16 +240,13 @@ impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::De
236240

237241
fn evaluate(&self, config: &[usize]) -> SolutionSize<f64> {
238242
let values = self.config_to_values(config);
239-
// Compute Bx - t, then ‖Bx - t‖₂
240243
let m = self.ambient_dimension();
241244
let mut diff = vec![0.0f64; m];
242-
// Bx = sum_i x_i * basis[i]
243245
for (i, &x_i) in values.iter().enumerate() {
244246
for (j, b_ji) in self.basis[i].iter().enumerate() {
245247
diff[j] += x_i as f64 * (*b_ji).clone().into();
246248
}
247249
}
248-
// diff = Bx - t
249250
for j in 0..m {
250251
diff[j] -= self.target[j];
251252
}
@@ -254,31 +255,37 @@ impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::De
254255
}
255256

256257
fn variant() -> Vec<(&'static str, &'static str)> {
257-
crate::variant_params![]
258-
}
259-
260-
fn problem_size_names() -> &'static [&'static str] {
261-
&["num_basis_vectors", "ambient_dimension"]
262-
}
263-
264-
fn problem_size_values(&self) -> Vec<usize> {
265-
vec![self.num_basis_vectors(), self.ambient_dimension()]
258+
crate::variant_params![T]
266259
}
267260
}
268261

269-
impl<T: Clone + Into<f64> + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static> OptimizationProblem for ClosestVectorProblem<T> {
262+
impl<T> OptimizationProblem for ClosestVectorProblem<T>
263+
where
264+
T: Clone + Into<f64> + crate::variant::VariantParam + Serialize + for<'de> Deserialize<'de> + std::fmt::Debug + 'static,
265+
{
270266
type Value = f64;
271267

272268
fn direction(&self) -> Direction {
273269
Direction::Minimize
274270
}
275271
}
276272

273+
crate::declare_variants! {
274+
ClosestVectorProblem<i32> => "exp(num_basis_vectors)",
275+
ClosestVectorProblem<f64> => "exp(num_basis_vectors)",
276+
}
277+
277278
#[cfg(test)]
278279
#[path = "../../unit_tests/models/optimization/closest_vector_problem.rs"]
279280
mod tests;
280281
```
281282

283+
**Notes on changes from original plan:**
284+
- **Added `crate::variant::VariantParam` trait bound** on `T` (required by refactored variant system, per Copilot review)
285+
- **Changed `variant_params![]` to `variant_params![T]`** since CVP is parameterized by element type `T` which maps to "weight" category (per Copilot review)
286+
- **Removed `problem_size_names()` / `problem_size_values()`** — these methods were removed from the `Problem` trait. Size getters are now inherent methods only (already have `num_basis_vectors()` and `ambient_dimension()`)
287+
- **Added `declare_variants!`** block registering both `i32` and `f64` concrete variants with complexity metadata. CVP complexity is `exp(num_basis_vectors)` — exact CVP is NP-hard under randomized reductions and the best known exact algorithms have exponential complexity in the lattice dimension
288+
282289
**Step 4: Run tests to verify they pass**
283290

284291
Run: `cargo test test_cvp -- --no-capture 2>&1 | tail -20`
@@ -344,14 +351,20 @@ Add import at top:
344351
use problemreductions::models::optimization::ClosestVectorProblem;
345352
```
346353

347-
Add match arm in `load_problem()` (after the `"ILP"` arm):
354+
Add match arm in `load_problem()` (after the `"ILP"` arm), supporting both i32 and f64 via variant map (following SpinGlass pattern, per Copilot review):
348355
```rust
349-
"ClosestVectorProblem" => deser_opt::<ClosestVectorProblem<i32>>(data),
356+
"ClosestVectorProblem" => match variant.get("weight").map(|s| s.as_str()) {
357+
Some("f64") => deser_opt::<ClosestVectorProblem<f64>>(data),
358+
_ => deser_opt::<ClosestVectorProblem<i32>>(data),
359+
},
350360
```
351361

352-
Add match arm in `serialize_any_problem()` (after the `"ILP"` arm):
362+
Add match arm in `serialize_any_problem()` (after the `"ILP"` arm), same pattern (per Copilot review):
353363
```rust
354-
"ClosestVectorProblem" => try_ser::<ClosestVectorProblem<i32>>(data),
364+
"ClosestVectorProblem" => match variant.get("weight").map(|s| s.as_str()) {
365+
Some("f64") => try_ser::<ClosestVectorProblem<f64>>(any),
366+
_ => try_ser::<ClosestVectorProblem<i32>>(any),
367+
},
355368
```
356369

357370
**Step 2: Update `problemreductions-cli/src/problem_name.rs`**
@@ -447,16 +460,6 @@ fn test_cvp_2d_identity() {
447460
assert_eq!(values, vec![0, 1]);
448461
}
449462

450-
#[test]
451-
fn test_cvp_problem_size() {
452-
let basis = vec![vec![1, 0, 0], vec![0, 1, 0]]; // 2 vectors in R^3
453-
let target = vec![0.5, 0.5, 0.5];
454-
let bounds = vec![VarBounds::bounded(0, 2), VarBounds::bounded(0, 2)];
455-
let cvp = ClosestVectorProblem::new(basis, target, bounds);
456-
assert_eq!(ClosestVectorProblem::<i32>::problem_size_names(), &["num_basis_vectors", "ambient_dimension"]);
457-
assert_eq!(cvp.problem_size_values(), vec![2, 3]);
458-
}
459-
460463
#[test]
461464
fn test_cvp_evaluate_exact_solution() {
462465
// Target is exactly a lattice point: t = (2, 2), basis = identity
@@ -529,7 +532,7 @@ Add a `#problem-def` block (after the ILP definition, in the optimization sectio
529532

530533
**Step 3: Verify paper builds**
531534

532-
Run: `make doc 2>&1 | tail -10`
535+
Run: `make paper 2>&1 | tail -10`
533536
Expected: successful build (warnings about missing reductions are OK for a new problem with no reduction rules yet)
534537

535538
**Step 4: Commit**

0 commit comments

Comments
 (0)