From 035a8fc8a4d6df01c1135c49f5d34feefbbdabb4 Mon Sep 17 00:00:00 2001 From: David Wang Date: Sat, 11 Mar 2017 08:37:29 +1100 Subject: [PATCH] Clean up: candidate manipulation returns a Result. Functions to manipulate candidates during the solution search now always return a Result instead of sometimes returning a bool, and other times returning an Option. We opted to use Result over Option mainly to (ab)use the try! macro. In addition, it also makes it emit a warning when we forget to handle the result. (We tend to assume people don't just ignore the result and continue as normal, instead of putting the puzzle in a special contradiction detected state.) --- src/constraint/alldifferent.rs | 28 ++--- src/constraint/equality.rs | 84 ++++++------- src/constraint/mod.rs | 21 ++-- src/lib.rs | 3 + src/puzzle.rs | 211 ++++++++++++++++++--------------- tests/queens.rs | 12 +- 6 files changed, 183 insertions(+), 176 deletions(-) diff --git a/src/constraint/alldifferent.rs b/src/constraint/alldifferent.rs index d9a6871..0deef62 100644 --- a/src/constraint/alldifferent.rs +++ b/src/constraint/alldifferent.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::rc::Rc; -use ::{Constraint,PuzzleSearch,Val,VarToken}; +use ::{Constraint,PsResult,PuzzleSearch,Val,VarToken}; pub struct AllDifferent { vars: Vec, @@ -34,18 +34,16 @@ impl Constraint for AllDifferent { Box::new(self.vars.iter()) } - fn on_assigned(&self, search: &mut PuzzleSearch, _var: VarToken, val: Val) - -> bool { - for &var2 in self.vars.iter() { - if !search.is_assigned(var2) { - search.remove_candidate(var2, val); - } + fn on_assigned(&self, search: &mut PuzzleSearch, var: VarToken, val: Val) + -> PsResult<()> { + for &var2 in self.vars.iter().filter(|&v| *v != var) { + try!(search.remove_candidate(var2, val)); } - true + Ok(()) } - fn on_updated(&self, search: &mut PuzzleSearch) -> bool { + fn on_updated(&self, search: &mut PuzzleSearch) -> PsResult<()> { // Build a table of which values can be assigned to which variables. let mut num_unassigned = 0; let mut all_candidates = HashMap::new(); @@ -64,30 +62,30 @@ impl Constraint for AllDifferent { if num_unassigned > all_candidates.len() { // More unassigned variables than candidates, contradiction. - return false; + return Err(()); } else if num_unassigned == all_candidates.len() { // As many as variables as candidates. for (&val, &opt) in all_candidates.iter() { if let Some(var) = opt { - search.set_candidate(var, val); + try!(search.set_candidate(var, val)); } } } - true + Ok(()) } fn substitute(&self, from: VarToken, to: VarToken) - -> Option> { + -> PsResult> { if let Some(idx) = self.vars.iter().position(|&var| var == from) { if !self.vars.contains(&to) { let mut new_vars = self.vars.clone(); new_vars[idx] = to; - return Some(Rc::new(AllDifferent{ vars: new_vars })); + return Ok(Rc::new(AllDifferent{ vars: new_vars })); } } - None + Err(()) } } diff --git a/src/constraint/equality.rs b/src/constraint/equality.rs index b35aaba..0312f4b 100644 --- a/src/constraint/equality.rs +++ b/src/constraint/equality.rs @@ -2,7 +2,7 @@ use std::rc::Rc; -use ::{Constraint,LinExpr,PuzzleSearch,Val,VarToken}; +use ::{Constraint,LinExpr,PsResult,PuzzleSearch,Val,VarToken}; use intdiv::IntDiv; pub struct Equality { @@ -36,7 +36,7 @@ impl Constraint for Equality { } fn on_assigned(&self, search: &mut PuzzleSearch, _: VarToken, _: Val) - -> bool { + -> PsResult<()> { let mut sum = self.eqn.constant; let mut unassigned_var = None; @@ -47,7 +47,7 @@ impl Constraint for Equality { // If we find more than one unassigned variable, // cannot assign any other variables. if unassigned_var.is_some() { - return true; + return Ok(()); } else { unassigned_var = Some((var, coef)); } @@ -59,34 +59,31 @@ impl Constraint for Equality { // sum + coef * var = 0. let val = -sum / coef; if sum + coef * val == 0 { - search.set_candidate(var, val); + try!(search.set_candidate(var, val)); } else { - return false; + return Err(()); } } else { if sum != 0 { - return false; + return Err(()); } } - true + Ok(()) } - fn on_updated(&self, search: &mut PuzzleSearch) -> bool { + fn on_updated(&self, search: &mut PuzzleSearch) -> PsResult<()> { let mut sum_min = self.eqn.constant; let mut sum_max = self.eqn.constant; for (&var, &coef) in self.eqn.coef.iter() { - if let Some((min_val, max_val)) = search.get_min_max(var) { - if coef > 0 { - sum_min += coef * min_val; - sum_max += coef * max_val; - } else { - sum_min += coef * max_val; - sum_max += coef * min_val; - } + let (min_val, max_val) = try!(search.get_min_max(var)); + if coef > 0 { + sum_min += coef * min_val; + sum_max += coef * max_val; } else { - return false; + sum_min += coef * max_val; + sum_max += coef * min_val; } } @@ -98,7 +95,7 @@ impl Constraint for Equality { while iters > 0 { iters = iters - 1; if !(sum_min <= 0 && 0 <= sum_max) { - return false; + return Err(()); } let (&var, &coef) = iter.next().expect("cycle"); @@ -106,51 +103,44 @@ impl Constraint for Equality { continue; } - if let Some((min_val, max_val)) = search.get_min_max(var) { - let min_bnd; - let max_bnd; + let (min_val, max_val) = try!(search.get_min_max(var)); + let (min_bnd, max_bnd); + + if coef > 0 { + min_bnd = (coef * max_val - sum_max).div_round_up(coef); + max_bnd = (coef * min_val - sum_min).div_round_down(coef); + } else { + min_bnd = (coef * max_val - sum_min).div_round_up(coef); + max_bnd = (coef * min_val - sum_max).div_round_down(coef); + } + + if min_val < min_bnd || max_bnd < max_val { + let (new_min, new_max) + = try!(search.bound_candidate_range(var, min_bnd, max_bnd)); if coef > 0 { - min_bnd = (coef * max_val - sum_max).div_round_up(coef); - max_bnd = (coef * min_val - sum_min).div_round_down(coef); + sum_min = sum_min + coef * (new_min - min_val); + sum_max = sum_max + coef * (new_max - max_val); } else { - min_bnd = (coef * max_val - sum_min).div_round_up(coef); - max_bnd = (coef * min_val - sum_max).div_round_down(coef); + sum_min = sum_min + coef * (new_max - max_val); + sum_max = sum_max + coef * (new_min - min_val); } - if min_val < min_bnd || max_bnd < max_val { - search.bound_candidate_range(var, min_bnd, max_bnd); - - if let Some((new_min, new_max)) = search.get_min_max(var) { - if coef > 0 { - sum_min = sum_min + coef * (new_min - min_val); - sum_max = sum_max + coef * (new_max - max_val); - } else { - sum_min = sum_min + coef * (new_max - max_val); - sum_max = sum_max + coef * (new_min - min_val); - } - } else { - return false; - } - - iters = self.eqn.coef.len(); - } - } else { - return false; + iters = self.eqn.coef.len(); } } - true + Ok(()) } fn substitute(&self, from: VarToken, to: VarToken) - -> Option> { + -> PsResult> { let mut eqn = self.eqn.clone(); if let Some(coef) = eqn.coef.remove(&from) { eqn = eqn + coef * to; } - Some(Rc::new(Equality{ eqn: eqn })) + Ok(Rc::new(Equality{ eqn: eqn })) } } diff --git a/src/constraint/mod.rs b/src/constraint/mod.rs index 3ae214f..27821ea 100644 --- a/src/constraint/mod.rs +++ b/src/constraint/mod.rs @@ -7,7 +7,7 @@ use std::rc::Rc; -use ::{PuzzleSearch,Val,VarToken}; +use ::{PsResult,PuzzleSearch,Val,VarToken}; /// Constraint trait. pub trait Constraint { @@ -15,29 +15,22 @@ pub trait Constraint { fn vars<'a>(&'a self) -> Box + 'a>; /// Applied after a variable has been assigned. - /// - /// Returns true if the search should continue with these variable - /// assignments, or false if the constraint found a contradiction. fn on_assigned(&self, _search: &mut PuzzleSearch, _var: VarToken, _val: Val) - -> bool { - true + -> PsResult<()> { + Ok(()) } /// Applied after a variable's candidates has been modified. - /// - /// Returns true if the search should continue with these variable - /// assignments, or false if the constraint found a contradiction. - fn on_updated(&self, _search: &mut PuzzleSearch) -> bool { - true + fn on_updated(&self, _search: &mut PuzzleSearch) -> PsResult<()> { + Ok(()) } /// Substitute the "from" variable with the "to" variable. /// /// Returns a new constraint with all instances of "from" replaced - /// with "to", or None if a contradiction was found in the - /// process. + /// with "to", or Err if a contradiction was found. fn substitute(&self, from: VarToken, to: VarToken) - -> Option>; + -> PsResult>; } pub use self::alldifferent::AllDifferent; diff --git a/src/lib.rs b/src/lib.rs index 4f41708..0218faf 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,9 @@ pub struct LinExpr { coef: HashMap, } +/// A result during a puzzle solution search (Err = contradiction). +pub type PsResult = Result; + /// A dictionary mapping puzzle variables to the solution value. #[derive(Debug)] pub struct Solution { diff --git a/src/puzzle.rs b/src/puzzle.rs index 965b0b4..5b06ee4 100644 --- a/src/puzzle.rs +++ b/src/puzzle.rs @@ -9,7 +9,7 @@ use std::ops::Index; use std::rc::Rc; use bit_set::BitSet; -use ::{Constraint,LinExpr,Solution,Val,VarToken}; +use ::{Constraint,LinExpr,PsResult,Solution,Val,VarToken}; use constraint; /// A collection of candidates. @@ -219,7 +219,6 @@ impl Puzzle { &Candidates::Set(_) => (), } - // Why you dumb Rust? if let Candidates::Set(ref mut rc) = self.candidates[idx] { let cs = Rc::get_mut(rc).expect("unique"); cs.extend(candidates); @@ -417,7 +416,7 @@ impl Puzzle { pub fn step(&mut self) -> Option { if self.num_vars > 0 { let mut search = PuzzleSearch::new(self); - if search.constrain() { + if search.constrain().is_ok() { return Some(search); } } @@ -486,105 +485,131 @@ impl<'a> PuzzleSearch<'a> { } /// Get the minimum and maximum values for variable. - pub fn get_min_max(&self, var: VarToken) -> Option<(Val, Val)> { + pub fn get_min_max(&self, var: VarToken) -> PsResult<(Val, Val)> { let VarToken(idx) = var; match &self.vars[idx] { - &VarState::Assigned(val) => Some((val, val)), + &VarState::Assigned(val) => Ok((val, val)), &VarState::Unassigned(ref cs) => match cs { - &Candidates::None => None, - &Candidates::Value(val) => Some((val, val)), + &Candidates::None => Err(()), + &Candidates::Value(val) => Ok((val, val)), &Candidates::Set(ref rc) => { rc.iter().cloned().min().into_iter() .zip(rc.iter().cloned().max()).next() + .ok_or(()) } }, } } - /// Set a variable to a known value. - pub fn set_candidate(&mut self, var: VarToken, val: Val) { + /// Set a variable to a value. + pub fn set_candidate(&mut self, var: VarToken, val: Val) + -> PsResult<()> { let VarToken(idx) = var; - match &mut self.vars[idx] { - &mut VarState::Assigned(_) => (), - &mut VarState::Unassigned(ref mut cs) => match cs { - &mut Candidates::None => return, - &mut Candidates::Value(v) => { - if v != val { - *cs = Candidates::None; - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, - &mut Candidates::Set(ref mut rc) => { - if rc.contains(&val) { - let mut set = Rc::make_mut(rc); - set.clear(); - set.insert(val); - self.wake.union_with(&self.puzzle.wake[idx]); - } else { - let mut set = Rc::make_mut(rc); - set.clear(); - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, + + match &self.vars[idx] { + &VarState::Assigned(v) => return bool_to_result(v == val), + &VarState::Unassigned(ref cs) => match cs { + &Candidates::None => return Err(()), + &Candidates::Value(v) => return bool_to_result(v == val), + &Candidates::Set(_) => (), }, } - } - /// Remove a single candidate from an unassigned variable. - pub fn remove_candidate(&mut self, var: VarToken, val: Val) { - let VarToken(idx) = var; - if let VarState::Unassigned(ref mut cs) = self.vars[idx] { - match cs { - &mut Candidates::None => return, - &mut Candidates::Value(v) => { - if v == val { - *cs = Candidates::None; - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, - &mut Candidates::Set(ref mut rc) => { - if rc.contains(&val) { - let mut set = Rc::make_mut(rc); - set.remove(&val); - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, + if let &mut VarState::Unassigned(Candidates::Set(ref mut rc)) + = &mut self.vars[idx] { + if rc.contains(&val) { + let mut set = Rc::make_mut(rc); + set.clear(); + set.insert(val); + self.wake.union_with(&self.puzzle.wake[idx]); + Ok(()) + } else { + Err(()) } + } else { + unreachable!(); } } - /// Bound an unassigned variable to the given range. - pub fn bound_candidate_range(&mut self, var: VarToken, min: Val, max: Val) { + /// Remove a single candidate from a variable. + pub fn remove_candidate(&mut self, var: VarToken, val: Val) + -> PsResult<()> { let VarToken(idx) = var; - if let VarState::Unassigned(ref mut cs) = self.vars[idx] { - match cs { - &mut Candidates::None => return, - &mut Candidates::Value(v) => { - if !(min <= v && v <= max) { - *cs = Candidates::None; - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, - &mut Candidates::Set(ref mut rc) => { - let &curr_min = rc.iter().min().expect("candidates"); - let &curr_max = rc.iter().max().expect("candidates"); - if curr_min < min || max < curr_max { - let mut set = Rc::make_mut(rc); - *set = set.iter() - .filter(|&val| min <= *val && *val <= max) - .cloned() - .collect(); - self.wake.union_with(&self.puzzle.wake[idx]); - } - }, + match &self.vars[idx] { + &VarState::Assigned(v) => return bool_to_result(v != val), + &VarState::Unassigned(ref cs) => match cs { + &Candidates::None => return Err(()), + &Candidates::Value(v) => return bool_to_result(v != val), + &Candidates::Set(_) => (), + }, + } + + if let &mut VarState::Unassigned(Candidates::Set(ref mut rc)) + = &mut self.vars[idx] { + if rc.contains(&val) { + let mut set = Rc::make_mut(rc); + set.remove(&val); + self.wake.union_with(&self.puzzle.wake[idx]); } + bool_to_result(!rc.is_empty()) + } else { + unreachable!(); + } + } + + /// Bound an variable to the given range. + pub fn bound_candidate_range(&mut self, var: VarToken, min: Val, max: Val) + -> PsResult<(Val, Val)> { + let VarToken(idx) = var; + + match &self.vars[idx] { + &VarState::Assigned(v) => + if min <= v && v <= max { + return Ok((v, v)) + } else { + return Err(()) + }, + &VarState::Unassigned(ref cs) => match cs { + &Candidates::None => return Err(()), + &Candidates::Value(v) => + if min <= v && v <= max { + return Ok((v, v)) + } else { + return Err(()) + }, + &Candidates::Set(_) => (), + }, + } + + if let &mut VarState::Unassigned(Candidates::Set(ref mut rc)) + = &mut self.vars[idx] { + let &curr_min = rc.iter().min().expect("candidates"); + let &curr_max = rc.iter().max().expect("candidates"); + + if curr_min < min || max < curr_max { + { + let mut set = Rc::make_mut(rc); + *set = set.iter() + .filter(|&val| min <= *val && *val <= max) + .cloned() + .collect(); + self.wake.union_with(&self.puzzle.wake[idx]); + } + rc.iter().cloned().min().into_iter() + .zip(rc.iter().cloned().max()).next() + .ok_or(()) + } else { + Ok((curr_min, curr_max)) + } + } else { + unreachable!(); } } /// Solve the puzzle, finding up to count solutions. fn solve(&mut self, count: usize, solutions: &mut Vec) { - if !self.constrain() { + if self.constrain().is_err() { return; } @@ -605,7 +630,7 @@ impl<'a> PuzzleSearch<'a> { self.puzzle.num_guesses.set(num_guesses); let mut new = self.clone(); - if !new.assign(idx, val) { + if new.assign(idx, val).is_err() { continue; } @@ -629,29 +654,23 @@ impl<'a> PuzzleSearch<'a> { } /// Assign a variable (given by index) to a value. - /// - /// Returns false if a contradiction was found. - fn assign(&mut self, idx: usize, val: Val) -> bool { + fn assign(&mut self, idx: usize, val: Val) -> PsResult<()> { let var = VarToken(idx); self.vars[idx] = VarState::Assigned(val); self.wake.union_with(&self.puzzle.wake[idx]); for (cidx, constraint) in self.puzzle.constraints.iter().enumerate() { if self.puzzle.wake[idx].contains(cidx) { - if !constraint.on_assigned(self, var, val) { - return false; - } + try!(constraint.on_assigned(self, var, val)); } } - true + Ok(()) } /// Take any obvious non-choices, using the constraints to /// eliminate candidates. Stops when it must start guessing. - /// - /// Returns false if a contradiction was found. - fn constrain(&mut self) -> bool { + fn constrain(&mut self) -> PsResult<()> { while !self.wake.is_empty() { // "Gimme" phase: // - abort if any variables with 0 candidates, @@ -664,16 +683,14 @@ impl<'a> PuzzleSearch<'a> { let gimme = match &self.vars[idx] { &VarState::Assigned(_) => None, &VarState::Unassigned(ref cs) => match cs.len() { - 0 => return false, + 0 => return Err(()), 1 => cs.iter().next(), _ => None, } }; if let Some(val) = gimme { - if !self.assign(idx, val) { - return false; - } + try!(self.assign(idx, val)); last_gimme = idx; } else if idx == last_gimme { break; @@ -686,14 +703,12 @@ impl<'a> PuzzleSearch<'a> { if !self.wake.is_empty() { let wake = mem::replace(&mut self.wake, BitSet::new()); for cidx in wake.iter() { - if !self.puzzle.constraints[cidx].on_updated(self) { - return false; - } + try!(self.puzzle.constraints[cidx].on_updated(self)); } } } - true + Ok(()) } } @@ -736,6 +751,14 @@ impl<'a> Index for PuzzleSearch<'a> { } } +fn bool_to_result(cond: bool) -> PsResult<()> { + if cond { + Ok(()) + } else { + Err(()) + } +} + #[cfg(test)] mod tests { use ::Puzzle; diff --git a/tests/queens.rs b/tests/queens.rs index 1fc60a0..2f59d44 100644 --- a/tests/queens.rs +++ b/tests/queens.rs @@ -5,7 +5,7 @@ extern crate puzzle_solver; use std::rc::Rc; -use puzzle_solver::{Constraint,Puzzle,PuzzleSearch,Solution,Val,VarToken}; +use puzzle_solver::*; struct NoDiagonal { vars: Vec, @@ -17,22 +17,22 @@ impl Constraint for NoDiagonal { } fn on_assigned(&self, search: &mut PuzzleSearch, var: VarToken, val: Val) - -> bool { + -> PsResult<()> { let y1 = self.vars.iter().position(|&v| v == var).expect("unreachable"); for (y2, &var2) in self.vars.iter().enumerate() { if !search.is_assigned(var2) { let x1 = val; let dy = (y1 as Val) - (y2 as Val); - search.remove_candidate(var2, x1 - dy); - search.remove_candidate(var2, x1 + dy); + try!(search.remove_candidate(var2, x1 - dy)); + try!(search.remove_candidate(var2, x1 + dy)); } } - true + Ok(()) } fn substitute(&self, _from: VarToken, _to: VarToken) - -> Option> { + -> PsResult> { unimplemented!(); } }