From 34545ad179c8719576820a7ffd324372848b1816 Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Mon, 28 Jun 2021 00:56:28 +1000 Subject: [PATCH] Check for ties when electing candidates with surpluses Refactor constraint-related code into constraints module --- src/constraints.rs | 104 ++++++++++++++++++++- src/main.rs | 2 +- src/stv/gregory.rs | 5 +- src/stv/meek.rs | 2 +- src/stv/mod.rs | 216 ++++++++++++++++--------------------------- src/stv/wasm.rs | 10 +- tests/constraints.rs | 6 +- tests/meek.rs | 4 +- tests/scotland.rs | 2 +- tests/utils/mod.rs | 2 +- 10 files changed, 200 insertions(+), 153 deletions(-) diff --git a/src/constraints.rs b/src/constraints.rs index 49e7432..d931c78 100644 --- a/src/constraints.rs +++ b/src/constraints.rs @@ -15,9 +15,11 @@ * along with this program. If not, see . */ -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, candidates: &HashMap<&Candidate, CountCard>, 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(state: &mut CountState, 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::*; diff --git a/src/main.rs b/src/main.rs index 89242c3..fe7f519 100644 --- a/src/main.rs +++ b/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); diff --git a/src/stv/gregory.rs b/src/stv/gregory.rs index 503e0e7..662fc3f 100644 --- a/src/stv/gregory.rs +++ b/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 diff --git a/src/stv/meek.rs b/src/stv/meek.rs index 2277056..0cbfeca 100644 --- a/src/stv/meek.rs +++ b/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; } diff --git a/src/stv/mod.rs b/src/stv/mod.rs index a6cd1e2..3cfa068 100644 --- a/src/stv/mod.rs +++ b/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 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); - calculate_quota(&mut state, opts); - elect_meeting_quota(&mut state, opts); - init_tiebreaks(&mut state, opts); + constraints::update_constraints(state, opts); + + distribute_first_preferences(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 +pub fn count_one_stage<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions) -> Result 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) { - calculate_quota(&mut state, opts); - elect_meeting_quota(&mut state, opts); - update_tiebreaks(&mut state, opts); + if continue_exclusion(state, &opts) { + calculate_quota(state, opts); + elect_meeting_quota(state, opts)?; + update_tiebreaks(state, opts); return Ok(false); } // Exclude doomed candidates - if exclude_doomed(&mut state, &opts)? { - calculate_quota(&mut state, opts); - elect_meeting_quota(&mut state, opts); - update_tiebreaks(&mut state, opts); + if exclude_doomed(state, &opts)? { + calculate_quota(state, opts); + elect_meeting_quota(state, opts)?; + update_tiebreaks(state, opts); return Ok(false); } // Distribute surpluses - if distribute_surpluses(&mut state, &opts)? { - calculate_quota(&mut state, opts); - elect_meeting_quota(&mut state, opts); - update_tiebreaks(&mut state, opts); + if distribute_surpluses(state, &opts)? { + calculate_quota(state, opts); + elect_meeting_quota(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)? { - calculate_quota(&mut state, opts); - elect_meeting_quota(&mut state, opts); - update_tiebreaks(&mut state, opts); + if exclude_hopefuls(state, &opts)? { + calculate_quota(state, opts); + elect_meeting_quota(state, opts)?; + update_tiebreaks(state, opts); return Ok(false); } @@ -728,10 +742,10 @@ fn meets_quota(quota: &N, count_card: &CountCard, opts: &STVOption } /// Declare elected all candidates meeting the quota -fn elect_meeting_quota(state: &mut CountState, opts: &STVOptions) -> bool { +fn elect_meeting_quota(state: &mut CountState, opts: &STVOptions) -> Result { 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(state: &mut CountState, 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(state: &mut CountState, 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(state: &mut CountState, opts: &STVOptions) if opts.quota_mode == QuotaMode::ERS97 { // Repeat in case vote required for election has changed - elect_meeting_quota(state, opts); - } - } - - return elected; -} - -fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election, candidates: &HashMap<&Candidate, CountCard>, 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; + match elect_meeting_quota(state, opts) { + Ok(_) => {} + Err(e) => { return Err(e); } } } - - if matches { - result.push(candidate); - } } - return result; -} - -fn update_constraints(state: &mut CountState, 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; + return Ok(elected); } /// Determine whether the transfer of all surpluses can be deferred @@ -970,9 +905,17 @@ fn bulk_elect(state: &mut CountState, opts: &STVOptions) -> Result vec![&candidate.name] ); - hopefuls.remove(hopefuls.iter().position(|c| *c == candidate).unwrap()); - - update_constraints(state, opts); + if constraints::update_constraints(state, opts) { + // Recheck as some candidates may have been doomed + 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(); - names.sort(); + 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( diff --git a/src/stv/wasm.rs b/src/stv/wasm.rs index 859beb8..248c7b0 100644 --- a/src/stv/wasm.rs +++ b/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 [](state: &mut [], opts: &STVOptions) { - stv::count_init(&mut state.0, opts.as_static()); + pub fn [](state: &mut [], opts: &STVOptions) -> Result { + 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 [](state: &mut [], opts: &STVOptions) -> Result { match stv::count_one_stage::<[<$type>]>(&mut state.0, &opts.0) { Ok(v) => Ok(v), - Err(stv::STVError::RequireInput) => Err("RequireInput".into()), - Err(stv::STVError::UnresolvedTie) => Err("UnresolvedTie".into()), + Err(e) => Err(e.name().into()), } } diff --git a/tests/constraints.rs b/tests/constraints.rs index 1432400..c219a1c 100644 --- a/tests/constraints.rs +++ b/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::(&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::(&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::(&mut state, &stv_opts).unwrap() == false {} // Validate winners diff --git a/tests/meek.rs b/tests/meek.rs index 083db3a..b2b137e 100644 --- a/tests/meek.rs +++ b/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::(&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::(&mut state, &stv_opts).unwrap() {} // Check states and keep values diff --git a/tests/scotland.rs b/tests/scotland.rs index 1542c56..4d496c7 100644 --- a/tests/scotland.rs +++ b/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 { diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 3f00171..9a73c9e 100644 --- a/tests/utils/mod.rs +++ b/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() {