Check for ties when electing candidates with surpluses

Refactor constraint-related code into constraints module

src/constraints.rs

@@ -15,9 +15,11 @@
  *  along with this program.  If not, see <https://www.gnu.org/licenses/>.
  */
 
-use crate::election::{Candidate, CandidateState, CountCard, Election};
+use crate::election::{Candidate, CandidateState, CountCard, CountState, Election};
 use crate::numbers::Number;
+use crate::stv::{ConstraintMode, STVOptions};
 
+use itertools::Itertools;
 use ndarray::{Array, Dimension, IxDyn};
 
 use std::collections::HashMap;
@@ -462,6 +464,106 @@ impl ops::IndexMut<&[usize]> for ConstraintMatrix {
 	fn index_mut(&mut self, index: &[usize]) -> &mut Self::Output { &mut self.0[index] }
 }
 
+/// Return the [Candidate]s referred to in the given [ConstraintMatrixCell] at location `idx`
+fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election<N>, candidates: &HashMap<&Candidate, CountCard<N>>, idx: &[usize]) -> Vec<&'a Candidate> {
+	let mut result: Vec<&Candidate> = Vec::new();
+	for (i, candidate) in election.candidates.iter().enumerate() {
+		let cc = candidates.get(candidate).unwrap();
+		if cc.state != CandidateState::Hopeful {
+			continue;
+		}
+		
+		// Is this candidate within this constraint cell?
+		let mut matches = true;
+		for (coord, constraint) in idx.iter().zip(election.constraints.as_ref().unwrap().0.iter()) {
+			let group = &constraint.groups[coord - 1]; // The group referred to by this constraint cell
+			if !group.candidates.contains(&i) {
+				matches = false;
+				break;
+			}
+		}
+		
+		if matches {
+			result.push(candidate);
+		}
+	}
+	
+	return result;
+}
+
+/// Update the constraints matrix, and perform the necessary actions given by [STVOptions::constraint_mode]
+pub fn update_constraints<N: Number>(state: &mut CountState<N>, opts: &STVOptions) -> bool {
+	if state.constraint_matrix.is_none() {
+		return false;
+	}
+	let cm = state.constraint_matrix.as_mut().unwrap();
+	
+	// Update cands/elected
+	cm.update_from_state(&state.election, &state.candidates);
+	cm.recount_cands();
+	
+	// Iterate for stable state
+	//println!("{}", cm);
+	while !cm.step().expect("No conformant result is possible") {
+		//println!("{}", cm);
+	}
+	//println!("{}", cm);
+	
+	match opts.constraint_mode {
+		ConstraintMode::GuardDoom => {
+			// Check for guarded or doomed candidates
+			let mut guarded_or_doomed = false;
+			
+			for idx in ndarray::indices(cm.0.shape()) {
+				if (0..idx.ndim()).fold(0, |acc, d| if idx[d] == 0 { acc + 1 } else { acc }) != 0 {
+					continue;
+				}
+				let cell = &cm.0[&idx];
+				
+				if cell.elected == cell.max {
+					// Doom remaining candidates in this cell
+					let doomed = candidates_in_constraint_cell(state.election, &state.candidates, idx.slice());
+					if !doomed.is_empty() {
+						for candidate in doomed.iter() {
+							state.candidates.get_mut(candidate).unwrap().state = CandidateState::Doomed;
+						}
+						
+						state.logger.log_smart(
+							"{} must be doomed to comply with constraints.",
+							"{} must be doomed to comply with constraints.",
+							doomed.iter().map(|c| c.name.as_str()).sorted().collect()
+						);
+						
+						guarded_or_doomed = true;
+					}
+				}
+				if cell.cands == cell.min {
+					// Guard remaining candidates in this cell
+					let guarded = candidates_in_constraint_cell(state.election, &state.candidates, idx.slice());
+					if !guarded.is_empty() {
+						for candidate in guarded.iter() {
+							state.candidates.get_mut(candidate).unwrap().state = CandidateState::Guarded;
+						}
+						
+						state.logger.log_smart(
+							"{} must be guarded to comply with constraints.",
+							"{} must be guarded to comply with constraints.",
+							guarded.iter().map(|c| c.name.as_str()).sorted().collect()
+						);
+						
+						guarded_or_doomed = true;
+					}
+				}
+			}
+			
+			return guarded_or_doomed;
+		}
+		_ => { todo!() }
+	}
+	
+	//return false;
+}
+
 #[cfg(test)]
 mod tests {
 	use super::*;

src/main.rs

@@ -284,7 +284,7 @@ where
 	let mut state = CountState::new(&election);
 	
 	// Distribute first preferences
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	let mut stage_num = 1;
 	make_and_print_result(stage_num, &state, &cmd_opts);
 	

src/stv/gregory.rs

@@ -17,6 +17,7 @@
 
 use super::{ExclusionMethod, NextPreferencesEntry, NextPreferencesResult, STVError, STVOptions, SumSurplusTransfersMode, SurplusMethod, SurplusOrder};
 
+use crate::constraints;
 use crate::election::{Candidate, CandidateState, CountCard, CountState, Parcel, Vote};
 use crate::numbers::Number;
 
@@ -361,7 +362,7 @@ where
 			state.num_excluded += 1;
 			count_card.order_elected = -(order_excluded as isize);
 			
-			super::update_constraints(state, opts);
+			constraints::update_constraints(state, opts);
 		}
 	}
 	
@@ -521,7 +522,7 @@ where
 			count_card.order_elected = -(order_excluded as isize);
 		}
 		
-		super::update_constraints(state, opts);
+		constraints::update_constraints(state, opts);
 	}
 	
 	// Reset count

src/stv/meek.rs

@@ -281,7 +281,7 @@ where
 			
 			if opts.meek_immediate_elect {
 				// Try to elect candidates
-				if super::elect_meeting_quota(state, opts) {
+				if super::elect_meeting_quota(state, opts)? {
 					candidates_elected = Some(state.logger.entries.pop().unwrap());
 					break;
 				}

src/stv/mod.rs

@@ -26,13 +26,13 @@ pub mod meek;
 //#[cfg(target_arch = "wasm32")]
 pub mod wasm;
 
+use crate::constraints;
 use crate::numbers::Number;
-use crate::election::{Candidate, CandidateState, CountCard, CountState, Election, Vote};
+use crate::election::{Candidate, CandidateState, CountCard, CountState, Vote};
 use crate::sharandom::SHARandom;
 use crate::ties::TieStrategy;
 
 use itertools::Itertools;
-use ndarray::Dimension;
 use wasm_bindgen::prelude::wasm_bindgen;
 
 use std::collections::HashMap;
@@ -432,8 +432,18 @@ pub enum STVError {
 	UnresolvedTie,
 }
 
+impl STVError {
+	/// Return the name of the error as a string
+	pub fn name(&self) -> &'static str {
+		match self {
+			STVError::RequireInput => "RequireInput",
+			STVError::UnresolvedTie => "UnresolvedTie",
+		}
+	}
+}
+
 /// Distribute first preferences, and initialise other states such as the random number generator and tie-breaking rules
-pub fn count_init<'a, N: Number>(mut state: &mut CountState<'a, N>, opts: &'a STVOptions)
+pub fn count_init<'a, N: Number>(state: &mut CountState<'a, N>, opts: &'a STVOptions) -> Result<bool, STVError>
 where
 	for<'r> &'r N: ops::Sub<&'r N, Output=N>,
 	for<'r> &'r N: ops::Mul<&'r N, Output=N>,
@@ -445,14 +455,18 @@ where
 		}
 	}
 	
-	distribute_first_preferences(&mut state, opts);
+	constraints::update_constraints(state, opts);
-	calculate_quota(&mut state, opts);
+	
-	elect_meeting_quota(&mut state, opts);
+	distribute_first_preferences(state, opts);
-	init_tiebreaks(&mut state, opts);
+	calculate_quota(state, opts);
+	elect_meeting_quota(state, opts)?;
+	init_tiebreaks(state, opts);
+	
+	return Ok(true);
 }
 
 /// Perform a single stage of the STV count
-pub fn count_one_stage<'a, N: Number>(mut state: &mut CountState<'a, N>, opts: &STVOptions) -> Result<bool, STVError>
+pub fn count_one_stage<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions) -> Result<bool, STVError>
 where
 	for<'r> &'r N: ops::Sub<&'r N, Output=N>,
 	for<'r> &'r N: ops::Mul<&'r N, Output=N>,
@@ -469,45 +483,45 @@ where
 	
 	// Attempt early bulk election
 	if opts.early_bulk_elect {
-		if bulk_elect(&mut state, &opts)? {
+		if bulk_elect(state, &opts)? {
 			return Ok(false);
 		}
 	}
 	
 	// Continue exclusions
-	if continue_exclusion(&mut state, &opts) {
+	if continue_exclusion(state, &opts) {
-		calculate_quota(&mut state, opts);
+		calculate_quota(state, opts);
-		elect_meeting_quota(&mut state, opts);
+		elect_meeting_quota(state, opts)?;
-		update_tiebreaks(&mut state, opts);
+		update_tiebreaks(state, opts);
 		return Ok(false);
 	}
 	
 	// Exclude doomed candidates
-	if exclude_doomed(&mut state, &opts)? {
+	if exclude_doomed(state, &opts)? {
-		calculate_quota(&mut state, opts);
+		calculate_quota(state, opts);
-		elect_meeting_quota(&mut state, opts);
+		elect_meeting_quota(state, opts)?;
-		update_tiebreaks(&mut state, opts);
+		update_tiebreaks(state, opts);
 		return Ok(false);
 	}
 	
 	// Distribute surpluses
-	if distribute_surpluses(&mut state, &opts)? {
+	if distribute_surpluses(state, &opts)? {
-		calculate_quota(&mut state, opts);
+		calculate_quota(state, opts);
-		elect_meeting_quota(&mut state, opts);
+		elect_meeting_quota(state, opts)?;
-		update_tiebreaks(&mut state, opts);
+		update_tiebreaks(state, opts);
 		return Ok(false);
 	}
 	
 	// Attempt late bulk election
-	if bulk_elect(&mut state, &opts)? {
+	if bulk_elect(state, &opts)? {
 		return Ok(false);
 	}
 	
 	// Exclude lowest hopeful
-	if exclude_hopefuls(&mut state, &opts)? {
+	if exclude_hopefuls(state, &opts)? {
-		calculate_quota(&mut state, opts);
+		calculate_quota(state, opts);
-		elect_meeting_quota(&mut state, opts);
+		elect_meeting_quota(state, opts)?;
-		update_tiebreaks(&mut state, opts);
+		update_tiebreaks(state, opts);
 		return Ok(false);
 	}
 	
@@ -728,10 +742,10 @@ fn meets_quota<N: Number>(quota: &N, count_card: &CountCard<N>, opts: &STVOption
 }
 
 /// Declare elected all candidates meeting the quota
-fn elect_meeting_quota<N: Number>(state: &mut CountState<N>, opts: &STVOptions) -> bool {
+fn elect_meeting_quota<N: Number>(state: &mut CountState<N>, opts: &STVOptions) -> Result<bool, STVError> {
 	let vote_req = state.vote_required_election.as_ref().unwrap().clone(); // Have to do this or else the borrow checker gets confused
 	
-	let mut cands_meeting_quota: Vec<&Candidate> = state.election.candidates.iter()
+	let mut cands_meeting_quota: Vec<&Candidate> = state.election.candidates.iter() // Present in order in case of tie
 		.filter(|c| {
 			let cc = state.candidates.get(c).unwrap();
 			return (cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) && meets_quota(&vote_req, cc, opts);
@@ -745,7 +759,22 @@ fn elect_meeting_quota<N: Number>(state: &mut CountState<N>, opts: &STVOptions)
 	
 	while !cands_meeting_quota.is_empty() {
 		// Declare elected in descending order of votes
-		let candidate = cands_meeting_quota.pop().unwrap();
+		let max_votes = cands_meeting_quota.iter()
+			.max_by(|a, b| state.candidates.get(**a).unwrap().votes.cmp(&state.candidates.get(**b).unwrap().votes))
+			.unwrap();
+		let max_votes = &state.candidates.get(max_votes).unwrap().votes;
+		let max_cands_meeting_quota: Vec<&Candidate> = cands_meeting_quota.iter()
+			.filter(|c| &state.candidates.get(**c).unwrap().votes == max_votes)
+			.map(|c| *c)
+			.collect();
+		
+		let candidate;
+		if max_cands_meeting_quota.len() > 1 {
+			// Handle ties
+			candidate = choose_highest(state, opts, max_cands_meeting_quota)?;
+		} else {
+			candidate = max_cands_meeting_quota[0];
+		}
 		
 		let count_card = state.candidates.get_mut(candidate).unwrap();
 		count_card.state = CandidateState::Elected;
@@ -757,12 +786,14 @@ fn elect_meeting_quota<N: Number>(state: &mut CountState<N>, opts: &STVOptions)
 			vec![&candidate.name]
 		);
 		
-		if update_constraints(state, opts) {
+		if constraints::update_constraints(state, opts) {
 			// Recheck as some candidates may have been doomed
 			cands_meeting_quota = state.election.candidates.iter()
 				.filter(|c| { let cc = state.candidates.get(c).unwrap(); cc.state == CandidateState::Hopeful && meets_quota(&vote_req, cc, opts) })
 				.collect();
 			cands_meeting_quota.sort_unstable_by(|a, b| state.candidates.get(a).unwrap().votes.cmp(&state.candidates.get(b).unwrap().votes));
+		} else {
+			cands_meeting_quota.remove(cands_meeting_quota.iter().position(|c| *c == candidate).unwrap());
 		}
 		
 		if opts.quota_mode == QuotaMode::ERS97 {
@@ -772,110 +803,14 @@ fn elect_meeting_quota<N: Number>(state: &mut CountState<N>, opts: &STVOptions)
 		
 		if opts.quota_mode == QuotaMode::ERS97 {
 			// Repeat in case vote required for election has changed
-			elect_meeting_quota(state, opts);
+			match elect_meeting_quota(state, opts) {
-		}
+				Ok(_) => {}
-	}
+				Err(e) => { return Err(e); }
-	
-	return elected;
-}
-
-fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election<N>, candidates: &HashMap<&Candidate, CountCard<N>>, idx: &[usize]) -> Vec<&'a Candidate> {
-	let mut result: Vec<&Candidate> = Vec::new();
-	for (i, candidate) in election.candidates.iter().enumerate() {
-		let cc = candidates.get(candidate).unwrap();
-		if cc.state != CandidateState::Hopeful {
-			continue;
-		}
-		
-		// Is this candidate within this constraint cell?
-		let mut matches = true;
-		for (coord, constraint) in idx.iter().zip(election.constraints.as_ref().unwrap().0.iter()) {
-			let group = &constraint.groups[coord - 1]; // The group referred to by this constraint cell
-			if !group.candidates.contains(&i) {
-				matches = false;
-				break;
 			}
 		}
-		
-		if matches {
-			result.push(candidate);
-		}
 	}
 	
-	return result;
+	return Ok(elected);
-}
-
-fn update_constraints<N: Number>(state: &mut CountState<N>, opts: &STVOptions) -> bool {
-	if state.constraint_matrix.is_none() {
-		return false;
-	}
-	let cm = state.constraint_matrix.as_mut().unwrap();
-	
-	// Update cands/elected
-	cm.update_from_state(&state.election, &state.candidates);
-	cm.recount_cands();
-	
-	// Iterate for stable state
-	//println!("{}", cm);
-	while !cm.step().expect("No conformant result") {
-		//println!("{}", cm);
-	}
-	//println!("{}", cm);
-	
-	// TODO: Refactor and move this to constraints module?
-	match opts.constraint_mode {
-		ConstraintMode::GuardDoom => {
-			// Check for guarded or doomed candidates
-			let mut guarded_or_doomed = false;
-			
-			for idx in ndarray::indices(cm.0.shape()) {
-				if (0..idx.ndim()).fold(0, |acc, d| if idx[d] == 0 { acc + 1 } else { acc }) != 0 {
-					continue;
-				}
-				let cell = &cm.0[&idx];
-				
-				if cell.elected == cell.max {
-					// Doom remaining candidates in this cell
-					let doomed = candidates_in_constraint_cell(state.election, &state.candidates, idx.slice());
-					if !doomed.is_empty() {
-						for candidate in doomed.iter() {
-							state.candidates.get_mut(candidate).unwrap().state = CandidateState::Doomed;
-						}
-						
-						state.logger.log_smart(
-							"{} must be doomed to comply with constraints.",
-							"{} must be doomed to comply with constraints.",
-							doomed.iter().map(|c| c.name.as_str()).collect()
-						);
-						
-						guarded_or_doomed = true;
-					}
-				}
-				if cell.cands == cell.min {
-					// Guard remaining candidates in this cell
-					let guarded = candidates_in_constraint_cell(state.election, &state.candidates, idx.slice());
-					if !guarded.is_empty() {
-						for candidate in guarded.iter() {
-							state.candidates.get_mut(candidate).unwrap().state = CandidateState::Guarded;
-						}
-						
-						state.logger.log_smart(
-							"{} must be guarded to comply with constraints.",
-							"{} must be guarded to comply with constraints.",
-							guarded.iter().map(|c| c.name.as_str()).collect()
-						);
-						
-						guarded_or_doomed = true;
-					}
-				}
-			}
-			
-			return guarded_or_doomed;
-		}
-		_ => { todo!() }
-	}
-	
-	//return false;
 }
 
 /// Determine whether the transfer of all surpluses can be deferred
@@ -970,9 +905,17 @@ fn bulk_elect<N: Number>(state: &mut CountState<N>, opts: &STVOptions) -> Result
 				vec![&candidate.name]
 			);
 			
-			hopefuls.remove(hopefuls.iter().position(|c| *c == candidate).unwrap());
+			if constraints::update_constraints(state, opts) {
-			
+				// Recheck as some candidates may have been doomed
-			update_constraints(state, opts);
+				hopefuls = state.election.candidates.iter()
+					.filter(|c| {
+						let cc = state.candidates.get(c).unwrap();
+						return cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded;
+					})
+					.collect();
+			} else {
+				hopefuls.remove(hopefuls.iter().position(|c| *c == candidate).unwrap());
+			}
 		}
 		
 		return Ok(true);
@@ -1011,7 +954,7 @@ where
 			}
 		}
 		
-		let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).collect();
+		let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect();
 		state.kind = Some("Exclusion of");
 		state.title = names.join(", ");
 		state.logger.log_smart(
@@ -1103,7 +1046,7 @@ where
 		}
 	}
 	
-	let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).collect();
+	let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect();
 	state.kind = Some("Exclusion of");
 	state.title = names.join(", ");
 	state.logger.log_smart(
@@ -1138,8 +1081,7 @@ where
 			.map(|(c, _)| *c)
 			.collect();
 		
-		let mut names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).collect();
+		let names: Vec<&str> = excluded_candidates.iter().map(|c| c.name.as_str()).sorted().collect();
-		names.sort();
 		state.kind = Some("Exclusion of");
 		state.title = names.join(", ");
 		state.logger.log_smart(

src/stv/wasm.rs

@@ -77,8 +77,11 @@ macro_rules! impl_type {
 		/// Wrapper for [stv::count_init]
 		#[wasm_bindgen]
 		#[allow(non_snake_case)]
-		pub fn [<count_init_$type>](state: &mut [<CountState$type>], opts: &STVOptions) {
+		pub fn [<count_init_$type>](state: &mut [<CountState$type>], opts: &STVOptions) -> Result<bool, JsValue> {
-			stv::count_init(&mut state.0, opts.as_static());
+			match stv::count_init(&mut state.0, opts.as_static()) {
+				Ok(v) => Ok(v),
+				Err(e) => Err(e.name().into()),
+			}
 		}
 		
 		/// Wrapper for [stv::count_one_stage]
@@ -87,8 +90,7 @@ macro_rules! impl_type {
 		pub fn [<count_one_stage_$type>](state: &mut [<CountState$type>], opts: &STVOptions) -> Result<bool, JsValue> {
 			match stv::count_one_stage::<[<$type>]>(&mut state.0, &opts.0) {
 				Ok(v) => Ok(v),
-				Err(stv::STVError::RequireInput) => Err("RequireInput".into()),
+				Err(e) => Err(e.name().into()),
-				Err(stv::STVError::UnresolvedTie) => Err("UnresolvedTie".into()),
 			}
 		}
 		

tests/constraints.rs

@@ -71,7 +71,7 @@ fn prsa1_constr1_rational() {
 	let mut state = CountState::new(&election);
 	
 	// Count election
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	while stv::count_one_stage::<Rational>(&mut state, &stv_opts).unwrap() == false {}
 	
 	// Validate winners
@@ -135,7 +135,7 @@ fn prsa1_constr2_rational() {
 	let mut state = CountState::new(&election);
 	
 	// Count election
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	while stv::count_one_stage::<Rational>(&mut state, &stv_opts).unwrap() == false {}
 	
 	// Validate winners
@@ -199,7 +199,7 @@ fn prsa1_constr3_rational() {
 	let mut state = CountState::new(&election);
 	
 	// Count election
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	while stv::count_one_stage::<Rational>(&mut state, &stv_opts).unwrap() == false {}
 	
 	// Validate winners

tests/meek.rs

@@ -96,7 +96,7 @@ fn meek06_ers97_fixed12() {
 	let mut state = CountState::new(&election);
 	
 	// Count to completion
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	while !stv::count_one_stage::<Fixed>(&mut state, &stv_opts).unwrap() {}
 	
 	// Check states and keep values
@@ -172,7 +172,7 @@ fn meeknz_ers97_fixed12() {
 	let mut state = CountState::new(&election);
 	
 	// Count to completion
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	while !stv::count_one_stage::<Fixed>(&mut state, &stv_opts).unwrap() {}
 	
 	// Check states and keep values

tests/scotland.rs

@@ -127,7 +127,7 @@ where
 	let mut state = CountState::new(&election);
 	
 	// Distribute first preferences
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	let mut stage_num = 1;
 	
 	for i in 0..num_stages {

tests/utils/mod.rs

@@ -78,7 +78,7 @@ where
 	let mut state = CountState::new(&election);
 	
 	// Distribute first preferences
-	stv::count_init(&mut state, &stv_opts);
+	stv::count_init(&mut state, &stv_opts).unwrap();
 	let mut stage_num = 1;
 	
 	for (idx, stage) in stages.into_iter().enumerate() {