From 4ad02c052ca88012a9f6c3118479d0db9670d213 Mon Sep 17 00:00:00 2001 From: RunasSudo Date: Sun, 21 Aug 2022 03:37:12 +1000 Subject: [PATCH] Refactor HashMap for Candidates into dedicated struct (CandidateMap) --- src/candmap.rs | 105 +++++++++++++++++++++++++++++++++++ src/constraints.rs | 11 ++-- src/election.rs | 17 +++--- src/lib.rs | 2 + src/stv/gregory/transfers.rs | 20 +++---- src/stv/meek.rs | 3 +- src/stv/mod.rs | 11 ++-- src/stv/sample.rs | 2 +- 8 files changed, 137 insertions(+), 34 deletions(-) create mode 100644 src/candmap.rs diff --git a/src/candmap.rs b/src/candmap.rs new file mode 100644 index 0000000..3bbfc0c --- /dev/null +++ b/src/candmap.rs @@ -0,0 +1,105 @@ +/* OpenTally: Open-source election vote counting + * Copyright © 2021–2022 Lee Yingtong Li (RunasSudo) + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + */ + +use crate::election::Candidate; + +use nohash_hasher::BuildNoHashHasher; + +use std::{collections::{HashMap, hash_map}, ops::Index}; + +/// Newtype for [HashMap] on [Candidate]s +#[derive(Clone)] +pub struct CandidateMap<'e, V> { + // TODO: Can we implement this more efficiently as a Vec? + map: HashMap<&'e Candidate, V, BuildNoHashHasher> +} + +impl<'e, V> CandidateMap<'e, V> { + /// See [HashMap::new] + pub fn new() -> Self { + Self { + map: HashMap::with_hasher(BuildNoHashHasher::default()) + } + } + + /// See [HashMap::with_capacity] + pub fn with_capacity(capacity: usize) -> Self { + Self { + map: HashMap::with_capacity_and_hasher(capacity, BuildNoHashHasher::default()) + } + } + + /// See [HashMap::len] + #[inline] + pub fn len(&self) -> usize { + return self.map.len(); + } + + /// See [HashMap::insert] + #[inline] + pub fn insert(&mut self, candidate: &'e Candidate, value: V) { + self.map.insert(candidate, value); + } + + /// See [HashMap::get] + #[inline] + pub fn get(&self, candidate: &'e Candidate) -> Option<&V> { + return self.map.get(candidate); + } + + /// See [HashMap::get_mut] + #[inline] + pub fn get_mut(&mut self, candidate: &'e Candidate) -> Option<&mut V> { + return self.map.get_mut(candidate); + } + + /// See [HashMap::iter] + #[inline] + pub fn iter(&self) -> hash_map::Iter<&'e Candidate, V> { + return self.map.iter(); + } + + /// See [HashMap::iter_mut] + #[inline] + pub fn iter_mut(&mut self) -> hash_map::IterMut<&'e Candidate, V> { + return self.map.iter_mut(); + } + + /// See [HashMap::values] + #[inline] + pub fn values(&self) -> hash_map::Values<&'e Candidate, V> { + return self.map.values(); + } +} + +impl<'e, V> Index<&Candidate> for CandidateMap<'e, V> { + type Output = V; + + fn index(&self, candidate: &Candidate) -> &Self::Output { + return &self.map[candidate]; + } +} + +impl<'e, V> IntoIterator for CandidateMap<'e, V> { + type Item = (&'e Candidate, V); + + type IntoIter = hash_map::IntoIter<&'e Candidate, V>; + + fn into_iter(self) -> Self::IntoIter { + return self.map.into_iter(); + } +} diff --git a/src/constraints.rs b/src/constraints.rs index d77f58e..57ac608 100644 --- a/src/constraints.rs +++ b/src/constraints.rs @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +use crate::candmap::CandidateMap; use crate::election::{Candidate, CandidateState, CountCard, CountState, Election, StageKind, RollbackState}; use crate::numbers::Number; use crate::stv::{self, gregory, sample, ConstraintMode, STVError, STVOptions, SurplusMethod, SurplusOrder}; @@ -22,12 +23,10 @@ use crate::ties::{self, TieStrategy}; use itertools::Itertools; use ndarray::{Array, Dimension, IxDyn}; -use nohash_hasher::BuildNoHashHasher; #[cfg(not(target_arch = "wasm32"))] use rkyv::{Archive, Deserialize, Serialize}; -use std::collections::HashMap; use std::fmt; use std::num::ParseIntError; use std::ops; @@ -131,7 +130,7 @@ impl Constraints { } /// Check if any elected candidates exceed constrained maximums - pub fn exceeds_maximum<'a, N: Number>(&self, election: &Election, candidates: HashMap<&'a Candidate, CountCard<'a, N>, BuildNoHashHasher>) -> Option<(&Constraint, &ConstrainedGroup)> { + pub fn exceeds_maximum<'a, N: Number>(&self, election: &Election, candidates: CandidateMap>) -> Option<(&Constraint, &ConstrainedGroup)> { for constraint in &self.0 { for group in &constraint.groups { let mut num_elected = 0; @@ -361,7 +360,7 @@ impl ConstraintMatrix { } /// Update cands/elected in innermost cells based on the provided [CountState::candidates](crate::election::CountState::candidates) - pub fn update_from_state(&mut self, election: &Election, candidates: &HashMap<&Candidate, CountCard, BuildNoHashHasher>) { + pub fn update_from_state(&mut self, election: &Election, candidates: &CandidateMap>) { let constraints = election.constraints.as_ref().unwrap(); // Reset innermost cells @@ -627,7 +626,7 @@ impl ops::IndexMut<&[usize]> for ConstraintMatrix { } /// 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, BuildNoHashHasher>, idx: &[usize]) -> Vec<&'a Candidate> { +fn candidates_in_constraint_cell<'a, N: Number>(election: &'a Election, candidates: &CandidateMap>, idx: &[usize]) -> Vec<&'a Candidate> { let mut result: Vec<&Candidate> = Vec::new(); for (i, candidate) in election.candidates.iter().enumerate() { let cc = &candidates[candidate]; @@ -784,7 +783,7 @@ pub fn init_repeat_count(election: &mut Election) { /// Initialise the rollback for [ConstraintMode::TwoStage] pub fn init_repeat_count_rollback<'a, N: Number>(state: &mut CountState<'a, N>, constraint: &'a Constraint, group: &'a ConstrainedGroup) { - let mut rollback_candidates = HashMap::with_capacity_and_hasher(state.candidates.len(), BuildNoHashHasher::default()); + let mut rollback_candidates = CandidateMap::with_capacity(state.candidates.len()); let rollback_exhausted = state.exhausted.clone(); // Copy ballot papers to rollback state diff --git a/src/election.rs b/src/election.rs index bb7c81e..40d51da 100644 --- a/src/election.rs +++ b/src/election.rs @@ -15,6 +15,7 @@ * along with this program. If not, see . */ +use crate::candmap::CandidateMap; use crate::constraints::{Constraint, Constraints, ConstrainedGroup, ConstraintMatrix}; use crate::logger::Logger; use crate::numbers::Number; @@ -24,7 +25,6 @@ use crate::stv::gregory::TransferTable; use crate::stv::meek::BallotTree; use itertools::Itertools; -use nohash_hasher::BuildNoHashHasher; #[cfg(not(target_arch = "wasm32"))] use rkyv::{Archive, Deserialize, Serialize}; @@ -32,7 +32,6 @@ use rkyv::{Archive, Deserialize, Serialize}; use crate::numbers::{SerializedNumber, SerializedOptionNumber}; use std::cmp::max; -use std::collections::HashMap; use std::fmt; use std::hash::{Hash, Hasher}; @@ -134,8 +133,8 @@ pub struct CountState<'a, N: Number> { /// Pointer to the [Election] being counted pub election: &'a Election, - /// [HashMap] of [CountCard]s for each [Candidate] in the election - pub candidates: HashMap<&'a Candidate, CountCard<'a, N>, BuildNoHashHasher>, + /// [CandidateMap] of [CountCard]s for each [Candidate] in the election + pub candidates: CandidateMap<'a, CountCard<'a, N>>, /// [CountCard] representing the exhausted pile pub exhausted: CountCard<'a, N>, /// [CountCard] representing loss by fraction @@ -145,9 +144,9 @@ pub struct CountState<'a, N: Number> { pub ballot_tree: Option>, /// Values used to break ties, based on forwards tie-breaking - pub forwards_tiebreak: Option>>, + pub forwards_tiebreak: Option>, /// Values used to break ties, based on backwards tie-breaking - pub backwards_tiebreak: Option>>, + pub backwards_tiebreak: Option>, /// [SHARandom] for random tie-breaking pub random: Option>, @@ -182,7 +181,7 @@ impl<'a, N: Number> CountState<'a, N> { pub fn new(election: &'a Election) -> Self { let mut state = CountState { election, - candidates: HashMap::with_capacity_and_hasher(election.candidates.len(), BuildNoHashHasher::default()), + candidates: CandidateMap::with_capacity(election.candidates.len()), exhausted: CountCard::new(), loss_fraction: CountCard::new(), ballot_tree: None, @@ -612,14 +611,14 @@ pub enum RollbackState<'a, N> { Normal, /// Start rolling back next stage NeedsRollback { - candidates: Option, BuildNoHashHasher>>, + candidates: Option>>, exhausted: Option>, constraint: &'a Constraint, group: &'a ConstrainedGroup }, /// Rolling back RollingBack { - candidates: Option, BuildNoHashHasher>>, + candidates: Option>>, exhausted: Option>, candidate_distributing: Option<&'a Candidate>, constraint: Option<&'a Constraint>, diff --git a/src/lib.rs b/src/lib.rs index 405c7b5..1bf3792 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,6 +20,8 @@ //! Open source counting software for various preferential voting election systems +/// Helper newtype for HashMap on [election::Candidate]s +pub mod candmap; /// Data types and logic for constraints on elections pub mod constraints; /// Data types for representing abstract elections diff --git a/src/stv/gregory/transfers.rs b/src/stv/gregory/transfers.rs index 4da058b..255bfd7 100644 --- a/src/stv/gregory/transfers.rs +++ b/src/stv/gregory/transfers.rs @@ -15,19 +15,17 @@ * along with this program. If not, see . */ -use nohash_hasher::BuildNoHashHasher; +use crate::candmap::CandidateMap; +use crate::election::{Candidate, CountState}; +use crate::numbers::Number; +use crate::stv::{STVOptions, RoundSubtransfersMode}; #[cfg(not(target_arch = "wasm32"))] use prettytable::{Cell, Row, Table}; #[cfg(target_arch = "wasm32")] use super::prettytable_html::{Cell, Row, Table}; -use crate::election::{Candidate, CountState}; -use crate::numbers::Number; -use crate::stv::{STVOptions, RoundSubtransfersMode}; - use std::cmp::max; -use std::collections::HashMap; /// Table describing vote transfers during a surplus distribution or exclusion #[derive(Clone)] @@ -62,7 +60,7 @@ impl<'e, N: Number> TransferTable<'e, N> { total: TransferTableColumn { value_fraction: N::new(), order: 0, - cells: HashMap::with_capacity_and_hasher(num_hopefuls, BuildNoHashHasher::default()), + cells: CandidateMap::with_capacity(num_hopefuls), exhausted: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, total: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, }, @@ -83,7 +81,7 @@ impl<'e, N: Number> TransferTable<'e, N> { total: TransferTableColumn { value_fraction: N::new(), order: 0, - cells: HashMap::with_capacity_and_hasher(num_hopefuls, BuildNoHashHasher::default()), + cells: CandidateMap::with_capacity(num_hopefuls), exhausted: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, total: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, }, @@ -108,7 +106,7 @@ impl<'e, N: Number> TransferTable<'e, N> { let mut col = TransferTableColumn { value_fraction: value_fraction.clone(), order: order.unwrap_or(0), - cells: HashMap::with_hasher(BuildNoHashHasher::default()), + cells: CandidateMap::new(), exhausted: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, total: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, }; @@ -130,7 +128,7 @@ impl<'e, N: Number> TransferTable<'e, N> { let col = TransferTableColumn { value_fraction: value_fraction.clone(), order: order.unwrap_or(0), - cells: HashMap::with_hasher(BuildNoHashHasher::default()), + cells: CandidateMap::new(), exhausted: TransferTableCell { ballots: ballots.clone(), votes_in: N::new(), votes_out: N::new() }, total: TransferTableCell { ballots: N::new(), votes_in: N::new(), votes_out: N::new() }, }; @@ -591,7 +589,7 @@ pub struct TransferTableColumn<'e, N: Number> { pub order: usize, /// Cells in this column - pub cells: HashMap<&'e Candidate, TransferTableCell, BuildNoHashHasher>, + pub cells: CandidateMap<'e, TransferTableCell>, /// Exhausted cell pub exhausted: TransferTableCell, diff --git a/src/stv/meek.rs b/src/stv/meek.rs index f0396c1..318187a 100644 --- a/src/stv/meek.rs +++ b/src/stv/meek.rs @@ -17,6 +17,7 @@ use super::{STVError, STVOptions}; +use crate::candmap::CandidateMap; use crate::election::{Ballot, Candidate, CandidateState, CountCard, CountState, Election, StageKind}; use crate::numbers::Number; @@ -165,7 +166,7 @@ where /// Distribute preferences recursively /// /// Called by [distribute_preferences] -fn distribute_recursively<'t, N: Number>(candidates: &mut HashMap<&'t Candidate, CountCard, BuildNoHashHasher>, exhausted: &mut CountCard, tree: &mut BallotTree<'t, N>, remaining_multiplier: N, election: &'t Election, opts: &STVOptions) +fn distribute_recursively<'t, N: Number>(candidates: &mut CandidateMap<'t, CountCard>, exhausted: &mut CountCard, tree: &mut BallotTree<'t, N>, remaining_multiplier: N, election: &'t Election, opts: &STVOptions) where for<'r> &'r N: ops::Mul<&'r N, Output=N>, { diff --git a/src/stv/mod.rs b/src/stv/mod.rs index b5e5a96..ee41ed0 100644 --- a/src/stv/mod.rs +++ b/src/stv/mod.rs @@ -26,6 +26,7 @@ pub mod sample; //#[cfg(target_arch = "wasm32")] pub mod wasm; +use crate::candmap::CandidateMap; use crate::constraints; use crate::numbers::Number; use crate::election::{Candidate, CandidateState, CountCard, CountState, Election, RollbackState, StageKind, Vote}; @@ -35,11 +36,9 @@ use crate::ties::{self, TieStrategy}; use derive_builder::Builder; use derive_more::Constructor; use itertools::Itertools; -use nohash_hasher::BuildNoHashHasher; #[allow(unused_imports)] use wasm_bindgen::prelude::wasm_bindgen; -use std::collections::HashMap; use std::fmt; use std::ops; @@ -782,7 +781,7 @@ where /// See [next_preferences] pub struct NextPreferencesResult<'a, N> { /// [NextPreferencesEntry] for each [Candidate] - pub candidates: HashMap<&'a Candidate, NextPreferencesEntry<'a, N>, BuildNoHashHasher>, + pub candidates: CandidateMap<'a, NextPreferencesEntry<'a, N>>, /// [NextPreferencesEntry] for exhausted ballots pub exhausted: NextPreferencesEntry<'a, N>, /// Total weight of ballots examined @@ -800,7 +799,7 @@ pub struct NextPreferencesEntry<'a, N> { /// Count the given votes, grouping according to next available preference pub fn next_preferences<'a, N: Number>(state: &CountState<'a, N>, votes: Vec>) -> NextPreferencesResult<'a, N> { let mut result = NextPreferencesResult { - candidates: HashMap::with_capacity_and_hasher(state.election.candidates.len(), BuildNoHashHasher::default()), + candidates: CandidateMap::with_capacity(state.election.candidates.len()), exhausted: NextPreferencesEntry { votes: Vec::new(), num_ballots: N::new(), @@ -1768,7 +1767,7 @@ fn init_tiebreaks(state: &mut CountState, opts: &STVOptions) { // Update forwards tie-breaking order if opts.ties.iter().any(|t| t == &TieStrategy::Forwards) { - let mut hm: HashMap<&Candidate, usize, BuildNoHashHasher> = HashMap::with_capacity_and_hasher(state.candidates.len(), BuildNoHashHasher::default()); + let mut hm = CandidateMap::with_capacity(state.candidates.len()); for (i, group) in sorted_candidates.iter().enumerate() { for (candidate, _) in group.iter() { hm.insert(candidate, i); @@ -1779,7 +1778,7 @@ fn init_tiebreaks(state: &mut CountState, opts: &STVOptions) { // Update backwards tie-breaking order if opts.ties.iter().any(|t| t == &TieStrategy::Backwards) { - let mut hm: HashMap<&Candidate, usize, BuildNoHashHasher> = HashMap::with_capacity_and_hasher(state.candidates.len(), BuildNoHashHasher::default()); + let mut hm = CandidateMap::with_capacity(state.candidates.len()); for (i, group) in sorted_candidates.iter().enumerate() { for (candidate, _) in group.iter() { hm.insert(candidate, i); diff --git a/src/stv/sample.rs b/src/stv/sample.rs index 065282a..d4696f7 100644 --- a/src/stv/sample.rs +++ b/src/stv/sample.rs @@ -372,7 +372,7 @@ where /// Transfer the given ballot paper to its next available preference, and check for candidates meeting the quota if --sample-per-ballot /// /// If `ignore_nontransferable`, does nothing if --transferable-only and the ballot is nontransferable. -fn transfer_ballot<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions, source_candidate: &Candidate, mut vote: Vote<'a, N>, ignore_nontransferable: bool) -> Result<(), STVError> { +fn transfer_ballot<'a, N: Number>(state: &mut CountState<'a, N>, opts: &STVOptions, source_candidate: &'a Candidate, mut vote: Vote<'a, N>, ignore_nontransferable: bool) -> Result<(), STVError> { // Get next preference let mut next_candidate = None; while let Some(preference) = vote.next_preference() {