Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: improve handling redistribution of votes #4

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 45 additions & 10 deletions packages/stv/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ describe('calculateStvWinners', () => {
];

const { winners, tieCount } = calculateStvWinners(voteRecords, 3);
expect(winners).toEqual(expect.arrayContaining(['Eve', 'Alice', 'Dave'])); // Eve, Alice, Dave should win
expect(winners).toEqual(expect.arrayContaining(['Eve', 'Alice', 'Bob'])); // Eve, Alice, Bob should win
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated test as the output changed.

expect(tieCount).toBe(0); // No tie expected
});

Expand Down Expand Up @@ -167,13 +167,13 @@ describe('calculateStvWinners', () => {

it('should correctly eliminate the candidate with the fewest votes', () => {
const voteRecords: VoteRecord[] = [
{ voteCount: 2, voteOrder: ['Alice', 'Bob'] },
{ voteCount: 1.5, voteOrder: ['Alice', 'Bob'] },
{ voteCount: 1, voteOrder: ['Charlie', 'Bob'] },
{ voteCount: 1, voteOrder: ['Bob', 'Charlie'] },
];

const { winners, tieCount } = calculateStvWinners(voteRecords, 1);
expect(winners).toEqual(['Alice']); // Alice should win after Charlie is eliminated
expect(winners).toEqual(['Bob']); // Bob should win after Charlie is eliminated
Comment on lines -170 to +176
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the test so it would actually trigger an elimination (I assume it did originally, but not in the updated code).

expect(tieCount).toBe(0);
});

Expand Down Expand Up @@ -212,13 +212,48 @@ describe('calculateStvWinners', () => {

it('should correctly redistribute votes when a candidate is eliminated', () => {
const voteRecords: VoteRecord[] = [
{ voteCount: 2, voteOrder: ['Alice', 'Charlie'] },
{ voteCount: 1, voteOrder: ['Bob', 'Alice'] },
{ voteCount: 1.2, voteOrder: ['Alice', 'Charlie'] },
{ voteCount: 1.1, voteOrder: ['Bob', 'Charlie'] },
{ voteCount: 1, voteOrder: ['Charlie', 'Bob'] },
{ voteCount: 0.9, voteOrder: ['Dave', 'Bob', 'Alice'] },
];

const { winners, tieCount } = calculateStvWinners(voteRecords, 2);
expect(winners).toEqual(['Bob', 'Alice']); // Bob wins first, followed by Alice
expect(tieCount).toBe(0);
});
Comment on lines -215 to +224
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the test so it would actually trigger an elimination (I assume it did originally, but not in the updated code).


it('should correctly handle a scenario where a winner has no backup candidates to assign excess votes to', () => {
// Quota values over time: 90, 30, 16 2/3
const voteRecords: VoteRecord[] = [
{ voteCount: 170, voteOrder: ['Alice'] },
{ voteCount: 10, voteOrder: ['Alice'] },
{ voteCount: 40, voteOrder: ['Bob'] },
{ voteCount: 25, voteOrder: ['Charlie'] },
{ voteCount: 10, voteOrder: ['Dave', 'Charlie'] },
{ voteCount: 10, voteOrder: ['Eve', 'Charlie'] },
{ voteCount: 5, voteOrder: ['Frank', 'Charlie'] },
];

const { winners, tieCount } = calculateStvWinners(voteRecords, 2);
expect(winners).toEqual(['Alice', 'Bob']); // Alice wins first, followed by Bob
expect(tieCount).toBe(0);
});

it('should correctly handle a scenario where a winner has a backup candidate defined for only a small portion of excess votes', () => {
// Quota values over time: 90, 33 1/3, 20
const voteRecords: VoteRecord[] = [
{ voteCount: 170, voteOrder: ['Alice'] },
{ voteCount: 10, voteOrder: ['Alice', 'Eve'] },
{ voteCount: 40, voteOrder: ['Bob'] },
{ voteCount: 25, voteOrder: ['Charlie'] },
{ voteCount: 10, voteOrder: ['Dave', 'Charlie'] },
{ voteCount: 10, voteOrder: ['Eve', 'Charlie'] },
{ voteCount: 5, voteOrder: ['Frank', 'Charlie'] },
];

const { winners, tieCount } = calculateStvWinners(voteRecords, 2);
expect(winners).toEqual(['Alice', 'Charlie']); // Alice wins first, followed by Charlie
expect(winners).toEqual(['Alice', 'Bob']); // Alice wins first, followed by Bob
expect(tieCount).toBe(0);
});
});
Expand Down Expand Up @@ -829,7 +864,7 @@ describe('redistributeToCandidates', () => {
{ totalVotes: 20, votes: [{ voteCount: 20, voteOrder: ['C', 'A'] }] },
],
]);
redistributeToCandidates(organizedVotes, candidateSet, 1, 10, 30);
redistributeToCandidates(organizedVotes, candidateSet, 1 / 3);
Comment on lines -832 to +867
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to update the next few tests as less input is passed to this method. This was also part of #3, but I had to do it here as there was just no need for the excess parameters anymore.

expect(candidateSet.get('B')?.totalVotes).toBeCloseTo(13.33, 2); // B should receive approximately 3.33 additional votes
expect(candidateSet.get('C')?.totalVotes).toBeCloseTo(26.67, 2); // C should receive approximately 6.67 additional votes
});
Expand All @@ -844,7 +879,7 @@ describe('redistributeToCandidates', () => {
{ totalVotes: 20, votes: [{ voteCount: 20, voteOrder: ['C', 'A'] }] },
],
]);
redistributeToCandidates(organizedVotes, candidateSet, 1, 10, 30);
redistributeToCandidates(organizedVotes, candidateSet, 1 / 3);
expect(candidateSet.has('C')).toBe(true);
expect(candidateSet.get('C')?.totalVotes).toBeCloseTo(6.67, 2); // C should receive approximately 6.67 additional votes
});
Expand All @@ -859,7 +894,7 @@ describe('redistributeToCandidates', () => {
{ totalVotes: 10, votes: [{ voteCount: 10, voteOrder: ['B', 'C'] }] },
],
]);
redistributeToCandidates(organizedVotes, candidateSet, 0.5, 10, 20);
redistributeToCandidates(organizedVotes, candidateSet, 0.5);
expect(candidateSet.get('B')?.totalVotes).toBe(15); // B should receive 5 additional votes
});

Expand All @@ -868,7 +903,7 @@ describe('redistributeToCandidates', () => {
['B', { totalVotes: 10, votes: [] }],
]);
const organizedVotes = new Map<Candidate, CandidateMapItem>();
redistributeToCandidates(organizedVotes, candidateSet, 1, 10, 20);
redistributeToCandidates(organizedVotes, candidateSet, 0.5);
expect(candidateSet.get('B')?.totalVotes).toBe(10); // No votes redistributed, B should remain the same
});
});
Expand Down
31 changes: 13 additions & 18 deletions packages/stv/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,28 +232,26 @@ export function redistributeExcessVotes(
for (const { totalVotes } of organizedVotes.values()) {
totalVotesForProportion += totalVotes;
}
const unassignedVotes = candidateData.totalVotes - totalVotesForProportion;

if (totalVotesForProportion === 0) {
if (unassignedVotes > 0) {
// console.log(
// `No votes to redistribute, reducing quota by ${candidateData.totalVotes} votes`,
// `Reducing quota by ${candidateData.totalVotes - totalVotesForProportion} votes to account for missing backup candidates`,
// );
newQuotaTotalVotes.value -= candidateData.totalVotes;
return;
newQuotaTotalVotes.value -= unassignedVotes;
}

if (candidateData.totalVotes === quota || totalVotesForProportion === 0) {
return; // No excess votes to redistribute, or no backup votes assigned
}

// Calculate the vote multiplier based on the excess votes
const votesToRedistribute = candidateData.totalVotes - quota;
const voteMultiplier = votesToRedistribute / totalVotesForProportion;
// console.log(`Redistributing ${votesToRedistribute} votes`);
const excessVotes = candidateData.totalVotes - quota;
const voteMultiplier = excessVotes / candidateData.totalVotes;
// console.log(`Redistributing ${excessVotes} excess votes`);

// Redistribute the votes proportionally
redistributeToCandidates(
organizedVotes,
candidateSet,
voteMultiplier,
votesToRedistribute,
totalVotesForProportion,
);
redistributeToCandidates(organizedVotes, candidateSet, voteMultiplier);
}

// Organize votes by the next candidate in the preference list
Expand Down Expand Up @@ -286,15 +284,12 @@ export function redistributeToCandidates(
organizedVotes: Map<Candidate, CandidateMapItem>,
candidateSet: Map<Candidate, CandidateMapItem>,
voteMultiplier: number,
votesToRedistribute: number,
totalVotesForProportion: number,
): void {
for (const [candidate, vote] of organizedVotes) {
vote.votes = combineVoteRecords(vote.votes);
vote.votes.forEach((v) => (v.voteCount *= voteMultiplier));

const votesToRedistributeForCandidate =
votesToRedistribute * (vote.totalVotes / totalVotesForProportion);
const votesToRedistributeForCandidate = vote.totalVotes * voteMultiplier;
Comment on lines -296 to +292
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been the case from the get-go, see also #3

It is necessary here though, as we want to ensure we use the updated voteMultiplier.

// console.log(
// `Redistributing ${votesToRedistributeForCandidate} votes to ${candidate}`,
// );
Expand Down