From 260dee1bb580b2c3512cc790e5646f49be899e5f Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Thu, 9 Sep 2021 01:19:31 +1000 Subject: [PATCH] Fix bugs Fix bug excluding-by-value/source candidates with no votes Fix bug electing too many candidates if more reach the quota than vacancies remain Add regression test --- src/stv/gregory.rs | 8 ++++-- src/stv/mod.rs | 7 +++-- tests/data/A33.blt | 17 +++++++++++ tests/{units.rs => special_cases.rs} | 43 +++++++++++++++++++++++++++- 4 files changed, 69 insertions(+), 6 deletions(-) create mode 100644 tests/data/A33.blt rename tests/{units.rs => special_cases.rs} (61%) diff --git a/src/stv/gregory.rs b/src/stv/gregory.rs index 5a58459..c41c047 100644 --- a/src/stv/gregory.rs +++ b/src/stv/gregory.rs @@ -496,7 +496,9 @@ where } ExclusionMethod::ByValue => { // Exclude by value - let excluded_with_votes: Vec<&&Candidate> = excluded_candidates.iter().filter(|c| !state.candidates[*c].finalised).collect(); + let excluded_with_votes: Vec<&&Candidate> = excluded_candidates.iter() + .filter(|c| { let cc = &state.candidates[*c]; !cc.finalised && !cc.parcels.is_empty() }) + .collect(); if excluded_with_votes.is_empty() { votes_remain = false; @@ -554,7 +556,9 @@ where } ExclusionMethod::BySource => { // Exclude by source candidate - let excluded_with_votes: Vec<&&Candidate> = excluded_candidates.iter().filter(|c| !state.candidates[*c].finalised).collect(); + let excluded_with_votes: Vec<&&Candidate> = excluded_candidates.iter() + .filter(|c| { let cc = &state.candidates[*c]; !cc.finalised && !cc.parcels.is_empty() }) + .collect(); if excluded_with_votes.is_empty() { votes_remain = false; diff --git a/src/stv/mod.rs b/src/stv/mod.rs index 5158c43..409fb06 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -939,11 +939,12 @@ fn meets_vre(state: &CountState, count_card: &CountCard, opts: /// /// Returns `true` if any candidates were elected. fn elect_sure_winners<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions) -> Result { - let num_vacancies = state.election.seats - state.num_elected; - if num_vacancies == 0 { + if state.num_elected >= state.election.seats { return Ok(false); } + let num_vacancies = state.election.seats - state.num_elected; + let mut hopefuls: Vec<(&Candidate, &CountCard)> = state.election.candidates.iter() .map(|c| (c, &state.candidates[c])) .filter(|(_, cc)| cc.state == CandidateState::Hopeful || cc.state == CandidateState::Guarded) @@ -1033,7 +1034,7 @@ fn elect_hopefuls<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOption let elected = !cands_meeting_quota.is_empty(); - while !cands_meeting_quota.is_empty() { + while !cands_meeting_quota.is_empty() && state.num_elected < state.election.seats { // Declare elected in descending order of votes let max_cands = ties::multiple_max_by(&cands_meeting_quota, |c| &state.candidates[c].votes); let candidate = if max_cands.len() > 1 { diff --git a/tests/data/A33.blt b/tests/data/A33.blt new file mode 100644 index 0000000..13d22d9 --- /dev/null +++ b/tests/data/A33.blt @@ -0,0 +1,17 @@ +# Comment: BY STV +# Comment: Number of truncated papers: 0 +# Comment: AKA R038 in Wichmann's database +# Source: Nicolaus Tideman via Warren D Smith +# Contributor: RunasSudo +18 3 +1 15 9 5 6 8 0 +1 18 7 16 1 12 14 0 +1 14 9 17 13 15 5 2 12 16 8 0 +1 6 5 3 4 15 12 9 7 1 14 13 16 18 17 8 2 10 11 0 +1 12 1 17 9 6 5 2 0 +1 15 8 5 3 0 +1 8 15 2 3 5 4 1 9 0 +1 3 5 1 4 11 2 8 0 +1 17 4 9 14 16 6 1 13 8 15 3 18 5 12 11 10 2 7 0 +0 +"A" "B" "C" "D" "E" "F" "G" "H" "I" "J" "K" "L" "M" "N" "O" "P" "Q" "R" "f:a33" diff --git a/tests/units.rs b/tests/special_cases.rs similarity index 61% rename from tests/units.rs rename to tests/special_cases.rs index 7a6ff78..753430f 100644 --- a/tests/units.rs +++ b/tests/special_cases.rs @@ -17,12 +17,13 @@ mod utils; -use opentally::election::{CountState, Election}; +use opentally::election::{CandidateState, CountState, Election}; use opentally::numbers::Rational; use opentally::parser::blt; use opentally::stv; use opentally::ties::TieStrategy; +/// Insufficient candidates to fill all vacancies #[test] fn insufficient_candidates1() { let stv_opts = stv::STVOptionsBuilder::default() @@ -48,6 +49,7 @@ fn insufficient_candidates1() { } } +/// After bulk exclusion of candidates with no votes, insufficient candidates remain to fill all vacancies #[test] fn insufficient_candidates2() { let stv_opts = stv::STVOptionsBuilder::default() @@ -72,3 +74,42 @@ fn insufficient_candidates2() { } } } + +/// Tideman A33 election: exclusion of candidate with no votes; more candidates reach the quota than vacancies +#[test] +fn tideman_a33_ers97_rational() { + let stv_opts = stv::STVOptionsBuilder::default() + .round_surplus_fractions(Some(2)) + .round_values(Some(2)) + .round_votes(Some(2)) + .round_quota(Some(2)) + .quota(stv::QuotaType::DroopExact) + .quota_criterion(stv::QuotaCriterion::GreaterOrEqual) + .quota_mode(stv::QuotaMode::ERS97) + .ties(vec![TieStrategy::Random(String::new("20210908"))]) + .surplus(stv::SurplusMethod::EG) + .transferable_only(true) + .exclusion(stv::ExclusionMethod::ByValue) + .early_bulk_elect(false) + .bulk_exclude(true) + .defer_surpluses(true) + .build().unwrap(); + + let mut election: Election = blt::parse_path("tests/data/A33.blt").expect("Syntax Error"); + stv::preprocess_election(&mut election, &stv_opts); + + let mut state = CountState::new(&election); + + stv::count_init(&mut state, &stv_opts).unwrap(); + + loop { + let result = stv::count_one_stage::(&mut state, &stv_opts); + match result { + Ok(done) => { if done { break; } } + Err(err) => { panic!("{}", err); } + } + } + + let num_winners = state.candidates.values().filter(|cc| cc.state == CandidateState::Elected).count(); + assert_eq!(num_winners, 3); +}