From cf7594382964bae8f2e71191416fe3d3820c637b Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Sun, 26 Sep 2021 02:27:37 +1000 Subject: [PATCH] Fixes to edge cases in stratify (LR) sample method --- src/stv/gregory/mod.rs | 2 +- src/stv/mod.rs | 16 ++++++++-------- src/stv/sample.rs | 31 +++++++++++++++++++++++++------ src/stv/wasm.rs | 12 ++++++------ 4 files changed, 40 insertions(+), 21 deletions(-) diff --git a/src/stv/gregory/mod.rs b/src/stv/gregory/mod.rs index 0488bb4..b5b850e 100644 --- a/src/stv/gregory/mod.rs +++ b/src/stv/gregory/mod.rs @@ -135,7 +135,7 @@ where } }; let elected_candidate = if max_cands.len() > 1 { - super::choose_highest(state, opts, max_cands, "Which candidate's surplus to distribute?")? + super::choose_highest(state, opts, &max_cands, "Which candidate's surplus to distribute?")? } else { max_cands[0] }; diff --git a/src/stv/mod.rs b/src/stv/mod.rs index 570b51d..6c48cc3 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -1061,7 +1061,7 @@ fn elect_sure_winners<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOp while !leading_hopefuls.is_empty() && state.num_elected < state.election.seats { let max_cands = ties::multiple_max_by(&leading_hopefuls, |c| &state.candidates[c].votes); let candidate = if max_cands.len() > 1 { - choose_highest(state, opts, max_cands, "Which candidate to elect?")? + choose_highest(state, opts, &max_cands, "Which candidate to elect?")? } else { max_cands[0] }; @@ -1109,7 +1109,7 @@ fn elect_hopefuls<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOption // 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 { - choose_highest(state, opts, max_cands, "Which candidate to elect?")? + choose_highest(state, opts, &max_cands, "Which candidate to elect?")? } else { max_cands[0] }; @@ -1207,7 +1207,7 @@ where if num_to_exclude > 0 { let total_excluded = to_exclude.into_iter() .fold(N::new(), |acc, c| acc + &state.candidates[c].votes); - if total_surpluses > &(&hopefuls[num_to_exclude + 1].1.votes - &total_excluded) { + if total_surpluses > &(&hopefuls[num_to_exclude].1.votes - &total_excluded) { return false; } } @@ -1265,7 +1265,7 @@ fn do_bulk_elect(state: &mut CountState, opts: &STVOptions, templa while !hopefuls.is_empty() { let max_cands = ties::multiple_max_by(&hopefuls, |c| &state.candidates[c].votes); let candidate = if max_cands.len() > 1 { - choose_highest(state, opts, max_cands, "Which candidate to elect?")? + choose_highest(state, opts, &max_cands, "Which candidate to elect?")? } else { max_cands[0] }; @@ -1330,7 +1330,7 @@ where // Exclude only the lowest-ranked doomed candidate let min_cands = ties::multiple_min_by(&doomed, |c| &state.candidates[c].votes); excluded_candidates = if min_cands.len() > 1 { - vec![choose_lowest(state, opts, min_cands, "Which candidate to exclude?")?] + vec![choose_lowest(state, opts, &min_cands, "Which candidate to exclude?")?] } else { vec![min_cands[0]] }; @@ -1468,7 +1468,7 @@ where let min_cands = ties::multiple_min_by(&hopefuls, |c| &state.candidates[c].votes); excluded_candidates = if min_cands.len() > 1 { - vec![choose_lowest(state, opts, min_cands, "Which candidate to exclude?")?] + vec![choose_lowest(state, opts, &min_cands, "Which candidate to exclude?")?] } else { vec![min_cands[0]] }; @@ -1586,7 +1586,7 @@ fn finished_before_stage(state: &CountState) -> bool { /// Break a tie between the given candidates according to [STVOptions::ties], selecting the highest candidate /// /// The given candidates are assumed to be tied in this round. -fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +pub fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { for strategy in opts.ties.iter() { match strategy.choose_highest(state, opts, &candidates, prompt_text) { Ok(c) => { @@ -1607,7 +1607,7 @@ fn choose_highest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, c /// Break a tie between the given candidates according to [STVOptions::ties], selecting the lowest candidate /// /// The given candidates are assumed to be tied in this round. -fn choose_lowest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { +pub fn choose_lowest<'c, N: Number>(state: &mut CountState, opts: &STVOptions, candidates: &Vec<&'c Candidate>, prompt_text: &str) -> Result<&'c Candidate, STVError> { for strategy in opts.ties.iter() { match strategy.choose_lowest(state, opts, &candidates, prompt_text) { Ok(c) => { diff --git a/src/stv/sample.rs b/src/stv/sample.rs index dd8dfc3..26eff6c 100644 --- a/src/stv/sample.rs +++ b/src/stv/sample.rs @@ -188,19 +188,38 @@ where // Round remainders to remove loss by fraction let transferred = candidate_transfers_remainders.values().fold(N::new(), |acc, (t, _)| acc + t); let loss_fraction = &surplus - &transferred; - if !loss_fraction.is_zero() { + if !loss_fraction.is_zero() && surplus_fraction.is_some() { let n_to_round: usize = format!("{:.0}", loss_fraction).parse().expect("Loss by fraction overflows usize"); - let mut cands_by_remainder: Vec> = candidate_transfers_remainders.keys().cloned().collect(); + let mut cands_by_remainder = candidate_transfers_remainders.keys().cloned().collect::>(); + + // Sort by whole parts // Compare b to a to sort high-low - cands_by_remainder.sort_unstable_by(|a, b| candidate_transfers_remainders[b].1.cmp(&candidate_transfers_remainders[a].1)); + cands_by_remainder.sort_unstable_by(|a, b| candidate_transfers_remainders[b].0.cmp(&candidate_transfers_remainders[a].0)); + + // Then sort by remainders + cands_by_remainder.sort_by(|a, b| candidate_transfers_remainders[b].1.cmp(&candidate_transfers_remainders[a].1)); // Select top remainders - let top_remainders: Vec<&Option<&Candidate>> = cands_by_remainder.iter().take(n_to_round).collect(); + let mut top_remainders = cands_by_remainder.iter().take(n_to_round).collect::>(); // Check for tied remainders - if candidate_transfers_remainders[top_remainders.last().unwrap()].1 == candidate_transfers_remainders[cands_by_remainder.iter().nth(n_to_round + 1).unwrap()].1 { - todo!("Tie for largest remainders"); + if candidate_transfers_remainders[top_remainders.last().unwrap()] == candidate_transfers_remainders[cands_by_remainder.iter().nth(n_to_round).unwrap()] { + // Get the top entry + let top_entry = &candidate_transfers_remainders[top_remainders.last().unwrap()]; + + // Separate out tied entries + top_remainders = top_remainders.into_iter().filter(|c| &candidate_transfers_remainders[c] != top_entry).collect(); + let mut tied_top = cands_by_remainder.iter() + .filter_map(|c| if let Some(c2) = c { if &candidate_transfers_remainders[c] == top_entry { Some(*c2) } else { None } } else { None }) + .collect::>(); + + // Get top entries by tie-breaking method + for _ in 0..(n_to_round-top_remainders.len()) { + let cand = super::choose_highest(state, opts, &tied_top, "Which fraction to round up?")?; + tied_top.remove(tied_top.iter().position(|c| *c == cand).unwrap()); + top_remainders.push(cands_by_remainder.iter().find(|c| **c == Some(cand)).unwrap()); + } } // Round up top remainders diff --git a/src/stv/wasm.rs b/src/stv/wasm.rs index 655aecd..2a4f5cf 100644 --- a/src/stv/wasm.rs +++ b/src/stv/wasm.rs @@ -309,7 +309,7 @@ impl STVOptions { // Reporting /// Generate the lead-in description of the count in HTML -fn describe_count(filename: String, election: &Election, opts: &stv::STVOptions) -> String { +pub fn describe_count(filename: String, election: &Election, opts: &stv::STVOptions) -> String { let mut result = String::from("

Count computed by OpenTally (revision "); result.push_str(crate::VERSION); let total_ballots = election.ballots.iter().fold(N::zero(), |acc, b| { acc + &b.orig_value }); @@ -326,7 +326,7 @@ fn describe_count(filename: String, election: &Election, opts: &st } /// Generate the first column of the HTML results table -fn init_results_table(election: &Election, opts: &stv::STVOptions, report_style: &str) -> String { +pub fn init_results_table(election: &Election, opts: &stv::STVOptions, report_style: &str) -> String { let mut result = String::from(r#""#); if report_style == "ballots_votes" { @@ -361,7 +361,7 @@ fn init_results_table(election: &Election, opts: &stv::STVOptions, } /// Generate subsequent columns of the HTML results table -fn update_results_table(stage_num: usize, state: &CountState, opts: &stv::STVOptions, report_style: &str) -> Array { +pub fn update_results_table(stage_num: usize, state: &CountState, opts: &stv::STVOptions, report_style: &str) -> Array { let result = Array::new(); // Insert borders to left of new exclusions in Wright STV @@ -621,7 +621,7 @@ fn update_results_table(stage_num: usize, state: &CountState, opts } /// Get the comment for the current stage -fn update_stage_comments(state: &CountState, stage_num: usize) -> String { +pub fn update_stage_comments(state: &CountState, stage_num: usize) -> String { let mut comments = state.logger.render().join(" "); if let Some(_) = state.transfer_table { comments.push_str(&format!(r##" [View detailed transfers]"##, stage_num)); @@ -630,7 +630,7 @@ fn update_stage_comments(state: &CountState, stage_num: usize) -> } /// Generate the final column of the HTML results table -fn finalise_results_table(state: &CountState, report_style: &str) -> Array { +pub fn finalise_results_table(state: &CountState, report_style: &str) -> Array { let result = Array::new(); // Header rows @@ -673,7 +673,7 @@ fn finalise_results_table(state: &CountState, report_style: &str) } /// Generate the final lead-out text summarising the result of the election -fn final_result_summary(state: &CountState, opts: &stv::STVOptions) -> String { +pub fn final_result_summary(state: &CountState, opts: &stv::STVOptions) -> String { let mut result = String::from("

Count complete. The winning candidates are, in order of election:

    "); let mut winners = Vec::new();