combine iteratons and tolerance in sp-npos-elections API (#11498)

* Initial implementation of mms

* Some more attempts at `mms`

* Functioning `MMS` algorithm implementation.
Adding some tests too

* More tests and typos fixed.

* Adding fuzzer for `mms`
(but could not test it on Mac M1)

* Missing imports

* Fixing rustdoc

* More accurate implementation of `mms`

* Removing the fuzzer `mms` implementation

* Implementing `NposSolver` for `MMS`
had to add the `Clone` trait, maybe I could see if I can get rid of it.

* Fixing rust docs by adding () to resolve ambiguity

* Amending `unwrap` to `expect`
removing unneeded `Clone` trait

* Removing redundant `mms3.rs`

* Implementing `BalancingConfig` and rustdoc changes

* Implementing `weight` for `MMS`

* Implementing `weight` for `MMS`

* Fixing post merge

* Initial implementation of mms

* Some more attempts at `mms`

* Functioning `MMS` algorithm implementation.
Adding some tests too

* More tests and typos fixed.

* Adding fuzzer for `mms`
(but could not test it on Mac M1)

* Missing imports

* Fixing rustdoc

* More accurate implementation of `mms`

* Removing the fuzzer `mms` implementation

* Implementing `NposSolver` for `MMS`
had to add the `Clone` trait, maybe I could see if I can get rid of it.

* Amending `unwrap` to `expect`
removing unneeded `Clone` trait

* Fixing rust docs by adding () to resolve ambiguity

* Removing redundant `mms3.rs`

* Implementing `BalancingConfig` and rustdoc changes

* Implementing `weight` for `MMS`

* Implementing `weight` for `MMS`

* Fixing post merge

* Removing left over from rebase

* Fixing tests

* Removing unneeded import

* Removing unneeded functions

* Removing useless imports

Co-authored-by: kianenigma <kian@parity.io>

substrate/bin/node/runtime/src/lib.rs

@@ -24,7 +24,7 @@
 
 use codec::{Decode, Encode, MaxEncodedLen};
 use frame_election_provider_support::{
-	onchain, ElectionDataProvider, ExtendedBalance, SequentialPhragmen, VoteWeight,
+	onchain, BalancingConfig, ElectionDataProvider, SequentialPhragmen, VoteWeight,
 };
 use frame_support::{
 	construct_runtime,
@@ -630,10 +630,10 @@ pub const MINER_MAX_ITERATIONS: u32 = 10;
 
 /// A source of random balance for NposSolver, which is meant to be run by the OCW election miner.
 pub struct OffchainRandomBalancing;
-impl Get<Option<(usize, ExtendedBalance)>> for OffchainRandomBalancing {
-	fn get() -> Option<(usize, ExtendedBalance)> {
+impl Get<Option<BalancingConfig>> for OffchainRandomBalancing {
+	fn get() -> Option<BalancingConfig> {
 		use sp_runtime::traits::TrailingZeroInput;
-		let iters = match MINER_MAX_ITERATIONS {
+		let iterations = match MINER_MAX_ITERATIONS {
 			0 => 0,
 			max => {
 				let seed = sp_io::offchain::random_seed();
@@ -644,7 +644,8 @@ impl Get<Option<(usize, ExtendedBalance)>> for OffchainRandomBalancing {
 			},
 		};
 
-		Some((iters, 0))
+		let config = BalancingConfig { iterations, tolerance: 0 };
+		Some(config)
 	}
 }
 

substrate/frame/election-provider-multi-phase/src/lib.rs

@@ -1808,7 +1808,7 @@ mod tests {
 	};
 	use frame_election_provider_support::ElectionProvider;
 	use frame_support::{assert_noop, assert_ok};
-	use sp_npos_elections::Support;
+	use sp_npos_elections::{BalancingConfig, Support};
 
 	#[test]
 	fn phase_rotation_works() {
@@ -2163,7 +2163,7 @@ mod tests {
 			assert_eq!(MultiPhase::current_phase(), Phase::Signed);
 
 			// set the solution balancing to get the desired score.
-			crate::mock::Balancing::set(Some((2, 0)));
+			crate::mock::Balancing::set(Some(BalancingConfig { iterations: 2, tolerance: 0 }));
 
 			let (solution, _) = MultiPhase::mine_solution().unwrap();
 			// Default solution's score.

substrate/frame/election-provider-multi-phase/src/mock.rs

@@ -39,8 +39,8 @@ use sp_core::{
 	H256,
 };
 use sp_npos_elections::{
-	assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, ElectionResult,
-	EvaluateSupport, ExtendedBalance,
+	assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, BalancingConfig,
+	ElectionResult, EvaluateSupport,
 };
 use sp_runtime::{
 	testing::Header,
@@ -324,7 +324,7 @@ impl InstantElectionProvider for MockFallback {
 }
 
 parameter_types! {
-	pub static Balancing: Option<(usize, ExtendedBalance)> = Some((0, 0));
+	pub static Balancing: Option<BalancingConfig> = Some( BalancingConfig { iterations: 0, tolerance: 0 } );
 }
 
 pub struct TestBenchmarkingConfig;

substrate/frame/election-provider-support/src/lib.rs

@@ -177,8 +177,8 @@ pub use frame_support::{traits::Get, weights::Weight, BoundedVec, RuntimeDebug};
 /// Re-export some type as they are used in the interface.
 pub use sp_arithmetic::PerThing;
 pub use sp_npos_elections::{
-	Assignment, ElectionResult, Error, ExtendedBalance, IdentifierT, PerThing128, Support,
-	Supports, VoteWeight,
+	Assignment, BalancingConfig, ElectionResult, Error, ExtendedBalance, IdentifierT, PerThing128,
+	Support, Supports, VoteWeight,
 };
 pub use traits::NposSolution;
 
@@ -568,11 +568,8 @@ pub struct SequentialPhragmen<AccountId, Accuracy, Balancing = ()>(
 	sp_std::marker::PhantomData<(AccountId, Accuracy, Balancing)>,
 );
 
-impl<
-		AccountId: IdentifierT,
-		Accuracy: PerThing128,
-		Balancing: Get<Option<(usize, ExtendedBalance)>>,
-	> NposSolver for SequentialPhragmen<AccountId, Accuracy, Balancing>
+impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<BalancingConfig>>>
+	NposSolver for SequentialPhragmen<AccountId, Accuracy, Balancing>
 {
 	type AccountId = AccountId;
 	type Accuracy = Accuracy;
@@ -596,11 +593,8 @@ pub struct PhragMMS<AccountId, Accuracy, Balancing = ()>(
 	sp_std::marker::PhantomData<(AccountId, Accuracy, Balancing)>,
 );
 
-impl<
-		AccountId: IdentifierT,
-		Accuracy: PerThing128,
-		Balancing: Get<Option<(usize, ExtendedBalance)>>,
-	> NposSolver for PhragMMS<AccountId, Accuracy, Balancing>
+impl<AccountId: IdentifierT, Accuracy: PerThing128, Balancing: Get<Option<BalancingConfig>>>
+	NposSolver for PhragMMS<AccountId, Accuracy, Balancing>
 {
 	type AccountId = AccountId;
 	type Accuracy = Accuracy;

substrate/frame/election-provider-support/src/weights.rs

@@ -15,15 +15,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! Autogenerated weights for pallet_election_provider_support_onchain_benchmarking
+//! Autogenerated weights for pallet_election_provider_support_benchmarking
 //!
 //! THIS FILE WAS AUTO-GENERATED USING THE SUBSTRATE BENCHMARK CLI VERSION 4.0.0-dev
-//! DATE: 2022-04-04, STEPS: `1`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]`
+//! DATE: 2022-04-23, STEPS: `1`, REPEAT: 1, LOW RANGE: `[]`, HIGH RANGE: `[]`
 //! EXECUTION: Some(Wasm), WASM-EXECUTION: Compiled, CHAIN: Some("dev"), DB CACHE: 1024
 
 // Executed Command:
 // target/release/substrate
 // benchmark
+// pallet
 // --chain=dev
 // --steps=1
 // --repeat=1

substrate/primitives/npos-elections/README.md

@@ -8,7 +8,7 @@ sub-system. Notable implementation include:
   it can achieve a constant factor approximation of the maximin problem, similar to that of the
   MMS algorithm.
 - [`balance_solution`]: Implements the star balancing algorithm. This iterative process can push
-  a solution toward being more `balances`, which in turn can increase its score.
+  a solution toward being more `balanced`, which in turn can increase its score.
 
 ### Terminology
 
@@ -46,7 +46,7 @@ let election_result = ElectionResult { winners, assignments };
 
 The `Assignment` field of the election result is voter-major, i.e. it is from the perspective of
 the voter. The struct that represents the opposite is called a `Support`. This struct is usually
-accessed in a map-like manner, i.e. keyed vy voters, therefor it is stored as a mapping called
+accessed in a map-like manner, i.e. keyed by voters, therefore it is stored as a mapping called
 `SupportMap`.
 
 Moreover, the support is built from absolute backing values, not ratios like the example above.

substrate/primitives/npos-elections/fuzzer/Cargo.toml

@@ -36,4 +36,4 @@ path = "src/phragmms_balancing.rs"
 
 [[bin]]
 name = "phragmen_pjr"
-path = "src/phragmen_pjr.rs"
+path = "src/phragmen_pjr.rs"

substrate/primitives/npos-elections/fuzzer/src/common.rs

@@ -21,7 +21,7 @@
 #![allow(dead_code)]
 
 use rand::{self, seq::SliceRandom, Rng, RngCore};
-use sp_npos_elections::{phragmms, seq_phragmen, ElectionResult, VoteWeight};
+use sp_npos_elections::{phragmms, seq_phragmen, BalancingConfig, ElectionResult, VoteWeight};
 use sp_runtime::Perbill;
 use std::collections::{BTreeMap, HashSet};
 
@@ -38,8 +38,8 @@ pub fn to_range(x: usize, a: usize, b: usize) -> usize {
 }
 
 pub enum ElectionType {
-	Phragmen(Option<(usize, u128)>),
-	Phragmms(Option<(usize, u128)>),
+	Phragmen(Option<BalancingConfig>),
+	Phragmms(Option<BalancingConfig>),
 }
 
 pub type AccountId = u64;

substrate/primitives/npos-elections/fuzzer/src/phragmen_balancing.rs

@@ -23,8 +23,8 @@ use common::*;
 use honggfuzz::fuzz;
 use rand::{self, SeedableRng};
 use sp_npos_elections::{
-	assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, ElectionResult,
-	EvaluateSupport, VoteWeight,
+	assignment_ratio_to_staked_normalized, seq_phragmen, to_supports, BalancingConfig,
+	ElectionResult, EvaluateSupport, VoteWeight,
 };
 use sp_runtime::Perbill;
 
@@ -66,8 +66,9 @@ fn main() {
 			};
 
 			if iterations > 0 {
+				let config = BalancingConfig { iterations, tolerance: 0 };
 				let balanced: ElectionResult<AccountId, sp_runtime::Perbill> =
-					seq_phragmen(to_elect, candidates, voters, Some((iterations, 0))).unwrap();
+					seq_phragmen(to_elect, candidates, voters, Some(config)).unwrap();
 
 				let balanced_score = {
 					let staked =

substrate/primitives/npos-elections/fuzzer/src/phragmms_balancing.rs

@@ -23,8 +23,8 @@ use common::*;
 use honggfuzz::fuzz;
 use rand::{self, SeedableRng};
 use sp_npos_elections::{
-	assignment_ratio_to_staked_normalized, phragmms, to_supports, ElectionResult, EvaluateSupport,
-	VoteWeight,
+	assignment_ratio_to_staked_normalized, phragmms, to_supports, BalancingConfig, ElectionResult,
+	EvaluateSupport, VoteWeight,
 };
 use sp_runtime::Perbill;
 
@@ -65,8 +65,9 @@ fn main() {
 				score
 			};
 
+			let config = BalancingConfig { iterations, tolerance: 0 };
 			let balanced: ElectionResult<AccountId, Perbill> =
-				phragmms(to_elect, candidates, voters, Some((iterations, 0))).unwrap();
+				phragmms(to_elect, candidates, voters, Some(config)).unwrap();
 
 			let balanced_score = {
 				let staked =

substrate/primitives/npos-elections/src/balancing.rs

@@ -26,14 +26,15 @@
 //!
 //! See [`balance`] for more information.
 
-use crate::{Edge, ExtendedBalance, IdentifierT, Voter};
+use crate::{BalancingConfig, Edge, ExtendedBalance, IdentifierT, Voter};
 use sp_arithmetic::traits::Zero;
 use sp_std::prelude::*;
 
 /// Balance the weight distribution of a given `voters` at most `iterations` times, or up until the
 /// point where the biggest difference created per iteration of all stakes is `tolerance`. If this
 /// is called with `tolerance = 0`, then exactly `iterations` rounds will be executed, except if no
-/// change has been made (`difference = 0`).
+/// change has been made (`difference = 0`). `tolerance` and `iterations` are part of the
+/// [`BalancingConfig`] struct.
 ///
 /// In almost all cases, a balanced solution will have a better score than an unbalanced solution,
 /// yet this is not 100% guaranteed because the first element of a [`crate::ElectionScore`] does not
@@ -52,12 +53,13 @@ use sp_std::prelude::*;
 /// - [A new approach to the maximum flow problem](https://dl.acm.org/doi/10.1145/48014.61051).
 /// - [Validator election in nominated proof-of-stake](https://arxiv.org/abs/2004.12990) (Appendix
 ///   A.)
+/// - [Computing a balanced solution](https://research.web3.foundation/en/latest/polkadot/NPoS/3.%20Balancing.html),
+///   which contains the details of the algorithm implementation.
 pub fn balance<AccountId: IdentifierT>(
 	voters: &mut Vec<Voter<AccountId>>,
-	iterations: usize,
-	tolerance: ExtendedBalance,
+	config: &BalancingConfig,
 ) -> usize {
-	if iterations == 0 {
+	if config.iterations == 0 {
 		return 0
 	}
 
@@ -65,14 +67,14 @@ pub fn balance<AccountId: IdentifierT>(
 	loop {
 		let mut max_diff = 0;
 		for voter in voters.iter_mut() {
-			let diff = balance_voter(voter, tolerance);
+			let diff = balance_voter(voter, config.tolerance);
 			if diff > max_diff {
 				max_diff = diff;
 			}
 		}
 
 		iter += 1;
-		if max_diff <= tolerance || iter >= iterations {
+		if max_diff <= config.tolerance || iter >= config.iterations {
 			break iter
 		}
 	}
@@ -158,7 +160,7 @@ pub(crate) fn balance_voter<AccountId: IdentifierT>(
 		.get(last_index)
 		.expect(
 			"length of elected_edges is greater than or equal 2; last_index index is at the \
-			 minimum elected_edges.len() - 1; index is within range; qed",
+ 			 minimum elected_edges.len() - 1; index is within range; qed",
 		)
 		.candidate
 		.borrow()

substrate/primitives/npos-elections/src/lib.rs

@@ -62,7 +62,7 @@
 //!
 //! The `Assignment` field of the election result is voter-major, i.e. it is from the perspective of
 //! the voter. The struct that represents the opposite is called a `Support`. This struct is usually
-//! accessed in a map-like manner, i.e. keyed by voters, therefor it is stored as a mapping called
+//! accessed in a map-like manner, i.e. keyed by voters, therefore it is stored as a mapping called
 //! `SupportMap`.
 //!
 //! Moreover, the support is built from absolute backing values, not ratios like the example above.
@@ -217,6 +217,13 @@ impl sp_std::cmp::PartialOrd for ElectionScore {
 	}
 }
 
+/// Utility struct to group parameters for the balancing algorithm.
+#[derive(Clone, Copy)]
+pub struct BalancingConfig {
+	pub iterations: usize,
+	pub tolerance: ExtendedBalance,
+}
+
 /// A pointer to a candidate struct with interior mutability.
 pub type CandidatePtr<A> = Rc<RefCell<Candidate<A>>>;
 
@@ -320,7 +327,7 @@ impl<AccountId: IdentifierT> Voter<AccountId> {
 	///
 	/// Note that this might create _un-normalized_ assignments, due to accuracy loss of `P`. Call
 	/// site might compensate by calling `normalize()` on the returned `Assignment` as a
-	/// post-precessing.
+	/// post-processing.
 	pub fn into_assignment<P: PerThing>(self) -> Option<Assignment<AccountId, P>> {
 		let who = self.who;
 		let budget = self.budget;

substrate/primitives/npos-elections/src/phragmen.rs

@@ -21,8 +21,8 @@
 //! to the Maximin problem.
 
 use crate::{
-	balancing, setup_inputs, CandidatePtr, ElectionResult, ExtendedBalance, IdentifierT,
-	PerThing128, VoteWeight, Voter,
+	balancing, setup_inputs, BalancingConfig, CandidatePtr, ElectionResult, ExtendedBalance,
+	IdentifierT, PerThing128, VoteWeight, Voter,
 };
 use sp_arithmetic::{
 	helpers_128bit::multiply_by_rational,
@@ -71,16 +71,16 @@ pub fn seq_phragmen<AccountId: IdentifierT, P: PerThing128>(
 	to_elect: usize,
 	candidates: Vec<AccountId>,
 	voters: Vec<(AccountId, VoteWeight, impl IntoIterator<Item = AccountId>)>,
-	balancing: Option<(usize, ExtendedBalance)>,
+	balancing: Option<BalancingConfig>,
 ) -> Result<ElectionResult<AccountId, P>, crate::Error> {
 	let (candidates, voters) = setup_inputs(candidates, voters);
 
 	let (candidates, mut voters) = seq_phragmen_core::<AccountId>(to_elect, candidates, voters)?;
 
-	if let Some((iterations, tolerance)) = balancing {
+	if let Some(ref config) = balancing {
 		// NOTE: might create zero-edges, but we will strip them again when we convert voter into
 		// assignment.
-		let _iters = balancing::balance::<AccountId>(&mut voters, iterations, tolerance);
+		let _iters = balancing::balance::<AccountId>(&mut voters, config);
 	}
 
 	let mut winners = candidates

substrate/primitives/npos-elections/src/phragmms.rs

@@ -22,15 +22,15 @@
 //! MMS algorithm.
 
 use crate::{
-	balance, setup_inputs, CandidatePtr, ElectionResult, ExtendedBalance, IdentifierT, PerThing128,
-	VoteWeight, Voter,
+	balance, setup_inputs, BalancingConfig, CandidatePtr, ElectionResult, ExtendedBalance,
+	IdentifierT, PerThing128, VoteWeight, Voter,
 };
 use sp_arithmetic::{traits::Bounded, PerThing, Rational128};
 use sp_std::{prelude::*, rc::Rc};
 
 /// Execute the phragmms method.
 ///
-/// This can be used interchangeably with [`seq-phragmen`] and offers a similar API, namely:
+/// This can be used interchangeably with `seq-phragmen` and offers a similar API, namely:
 ///
 /// - The resulting edge weight distribution is normalized (thus, safe to use for submission).
 /// - The accuracy can be configured via the generic type `P`.
@@ -45,7 +45,7 @@ pub fn phragmms<AccountId: IdentifierT, P: PerThing128>(
 	to_elect: usize,
 	candidates: Vec<AccountId>,
 	voters: Vec<(AccountId, VoteWeight, impl IntoIterator<Item = AccountId>)>,
-	balancing: Option<(usize, ExtendedBalance)>,
+	balancing: Option<BalancingConfig>,
 ) -> Result<ElectionResult<AccountId, P>, crate::Error> {
 	let (candidates, mut voters) = setup_inputs(candidates, voters);
 
@@ -58,8 +58,8 @@ pub fn phragmms<AccountId: IdentifierT, P: PerThing128>(
 			round_winner.borrow_mut().elected = true;
 			winners.push(round_winner);
 
-			if let Some((iterations, tolerance)) = balancing {
-				balance(&mut voters, iterations, tolerance);
+			if let Some(ref config) = balancing {
+				balance(&mut voters, config);
 			}
 		} else {
 			break
@@ -275,7 +275,8 @@ mod tests {
 		drop(winner);
 
 		// balancing makes no difference here but anyhow.
-		balance(&mut voters, 10, 0);
+		let config = BalancingConfig { iterations: 10, tolerance: 0 };
+		balance(&mut voters, &config);
 
 		// round 2
 		let winner =
@@ -315,7 +316,7 @@ mod tests {
 		drop(winner);
 
 		// balancing will improve stuff here.
-		balance(&mut voters, 10, 0);
+		balance(&mut voters, &config);
 
 		assert_eq!(
 			voters
@@ -348,8 +349,9 @@ mod tests {
 		let candidates = vec![1, 2, 3];
 		let voters = vec![(10, 10, vec![1, 2]), (20, 20, vec![1, 3]), (30, 30, vec![2, 3])];
 
+		let config = BalancingConfig { iterations: 2, tolerance: 0 };
 		let ElectionResult::<_, Perbill> { winners, assignments } =
-			phragmms(2, candidates, voters, Some((2, 0))).unwrap();
+			phragmms(2, candidates, voters, Some(config)).unwrap();
 		assert_eq!(winners, vec![(3, 30), (2, 30)]);
 		assert_eq!(
 			assignments,
@@ -380,8 +382,9 @@ mod tests {
 			(130, 1000, vec![61, 71]),
 		];
 
+		let config = BalancingConfig { iterations: 2, tolerance: 0 };
 		let ElectionResult::<_, Perbill> { winners, assignments: _ } =
-			phragmms(4, candidates, voters, Some((2, 0))).unwrap();
+			phragmms(4, candidates, voters, Some(config)).unwrap();
 		assert_eq!(winners, vec![(11, 3000), (31, 2000), (51, 1500), (61, 1500),]);
 	}
 
@@ -393,8 +396,9 @@ mod tests {
 		// give a bit more to 1 and 3.
 		voters.push((2, u64::MAX, vec![1, 3]));
 
+		let config = BalancingConfig { iterations: 2, tolerance: 0 };
 		let ElectionResult::<_, Perbill> { winners, assignments: _ } =
-			phragmms(2, candidates, voters, Some((2, 0))).unwrap();
+			phragmms(2, candidates, voters, Some(config)).unwrap();
 		assert_eq!(winners.into_iter().map(|(w, _)| w).collect::<Vec<_>>(), vec![1u32, 3]);
 	}
 }

substrate/primitives/npos-elections/src/tests.rs

@@ -19,7 +19,7 @@
 
 use crate::{
 	balancing, helpers::*, mock::*, seq_phragmen, seq_phragmen_core, setup_inputs, to_support_map,
-	Assignment, ElectionResult, ExtendedBalance, StakedAssignment, Support, Voter,
+	Assignment, BalancingConfig, ElectionResult, ExtendedBalance, StakedAssignment, Support, Voter,
 };
 use sp_arithmetic::{PerU16, Perbill, Percent, Permill};
 use substrate_test_utils::assert_eq_uvec;
@@ -142,7 +142,8 @@ fn balancing_core_works() {
 
 	let (candidates, voters) = setup_inputs(candidates, voters);
 	let (candidates, mut voters) = seq_phragmen_core(4, candidates, voters).unwrap();
-	let iters = balancing::balance::<AccountId>(&mut voters, 4, 0);
+	let config = BalancingConfig { iterations: 4, tolerance: 0 };
+	let iters = balancing::balance::<AccountId>(&mut voters, &config);
 
 	assert!(iters > 0);
 
@@ -282,6 +283,7 @@ fn phragmen_poc_works_with_balancing() {
 	let voters = vec![(10, vec![1, 2]), (20, vec![1, 3]), (30, vec![2, 3])];
 
 	let stake_of = create_stake_of(&[(10, 10), (20, 20), (30, 30)]);
+	let config = BalancingConfig { iterations: 4, tolerance: 0 };
 	let ElectionResult::<_, Perbill> { winners, assignments } = seq_phragmen(
 		2,
 		candidates,
@@ -289,7 +291,7 @@ fn phragmen_poc_works_with_balancing() {
 			.iter()
 			.map(|(ref v, ref vs)| (v.clone(), stake_of(v), vs.clone()))
 			.collect::<Vec<_>>(),
-		Some((4, 0)),
+		Some(config),
 	)
 	.unwrap();