Clean Phragmén Equlise API #5452
Conversation
kianenigma
added
A0-please_review
|
polkadot apparently doesn't build. |
|
all ✅ |
gavofyork
added
A6-mustntgrumble
and removed
A0-please_review
frame/elections-phragmen/src/lib.rs
Outdated
| 0, | ||
| candidates, | ||
| voters_and_votes.clone(), | ||
| voters_and_votes.iter().cloned().map(|(v, s, t)| (v, to_votes(s), t)).collect::<Vec<_>>(), |
There was a problem hiding this comment.
@gavofyork if it is okay to move your prime election code to use u64 (VoteWeight) instead of BalanceOf<T> then this extra clone of all voters can be removed.
There was a problem hiding this comment.
I've changed it for now, correct me if it is wrong.
| let to_range = |x: usize, a: usize, b: usize| { | ||
| let collapsed = x % b; | ||
| if collapsed >= a { | ||
| collapsed | ||
| } else { | ||
| collapsed + a | ||
| } | ||
| }; |
There was a problem hiding this comment.
the result can be more than b if b < a*2
There was a problem hiding this comment.
yeah true, this is slightly wrong.
There was a problem hiding this comment.
for now since all ranges are bigger than the mentioned range, I'll add an assertion to prevent this. I'd be interested to devise a minimum, correct and fair algorithm for this though.
| target_count = to_range(target_count, 50, 2000); | ||
| voter_count = to_range(voter_count, 50, 1000); | ||
| iterations = to_range(iterations, 1, 20); | ||
| to_elect = to_range(to_elect, 50, target_count); |
There was a problem hiding this comment.
this to_range can result in to_elect > target_count, is that wanted?
(due to #5452 (comment))
There was a problem hiding this comment.
nope, this should not happen and can cause false negatives, temporarily fixed.
Co-Authored-By: thiolliere <[email protected]>
|
I've no idea what this polkadot build error is. |
This code was dead for a while and needed a shake.
phragmenand callers is now standardised as: any crate calling intophragmenmust pass in vote weights (aka stakes) asu64(potentially via aT::CurrencyToVote).phragmenthen converts this value tou128and performs the calculations safely. This type is namedVoteWeight. Consequently, we don't pass around someT::CurrencyToVoteeverywhere.compactstill use closures.equalise()always gives a better solution.