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: Add Method of Equal Shares #599

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

fsargent
Copy link
Collaborator

Context:

This pull request adds new test cases for the MethodOfEqualShares class within the packages/backend/src/Tabulators/MethodOfEqualShares.ts file. This includes importing the fraction.js package and several testing scenarios such as Basic Example, Influence Budget Redistribution, Random Tiebreaker, Random Tiebreaker with Defined Order, and Valid/Invalid/Under/Bullet Vote Counts.

Key Changes:

  1. New test file:

    • Created a new test file at packages/backend/src/Tabulators/MethodOfEqualShares.test.ts.
    • Added comprehensive test cases to validate the functionality of the MethodOfEqualShares.
  2. Implementation of MethodOfEqualShares

    • Added packages/backend/src/Tabulators/MethodOfEqualShares.ts to implement the MethodOfEqualShares function.
    • The function calculates election results based on various voting methods and scenarios.
    • It handles tiebreakers, influence budget redistribution, and validates different vote types.

Testing Instructions:

  • Run the test suite in the packages/backend/ directory to ensure all new and existing tests pass.

Motivation:

  • To ensure the reliability and accuracy of the MethodOfEqualShares function.
  • To handle edge cases and validate the workflow of the election model.

fsargent added 2 commits July 13, 2024 21:41
**Context:**

This pull request adds new test cases for the `MethodOfEqualShares` class within the `packages/backend/src/Tabulators/MethodOfEqualShares.ts` file. This includes importing the `fraction.js` package and several testing scenarios such as Basic Example, Influence Budget Redistribution, Random Tiebreaker, Random Tiebreaker with Defined Order, and Valid/Invalid/Under/Bullet Vote Counts.

**Key Changes:**

1. **New test file:**
   - Created a new test file at `packages/backend/src/Tabulators/MethodOfEqualShares.test.ts`.
   - Added comprehensive test cases to validate the functionality of the `MethodOfEqualShares`.

2. **Implementation of MethodOfEqualShares**
   - Added `packages/backend/src/Tabulators/MethodOfEqualShares.ts` to implement the MethodOfEqualShares function.
   - The function calculates election results based on various voting methods and scenarios.
   - It handles tiebreakers, influence budget redistribution, and validates different vote types.

**Testing Instructions:**

- Run the test suite in the `packages/backend/` directory to ensure all new and existing tests pass.

**Motivation:**

- To ensure the reliability and accuracy of the `MethodOfEqualShares` function.
- To handle edge cases and validate the workflow of the election model.
@fsargent fsargent force-pushed the feat_add_MethodOfEqualShares branch 2 times, most recently from 6ce75b7 to 7f5a499 Compare July 18, 2024 17:26
@fsargent fsargent force-pushed the feat_add_MethodOfEqualShares branch from 7f5a499 to 512e788 Compare July 18, 2024 17:28
@fsargent
Copy link
Collaborator Author

fsargent commented Jul 18, 2024

image


 FAIL  src/Tabulators/MethodOfEqualShares.test.ts
  Method of Equal Shares Tests
    ✓ Basic Example (3 ms)
    ✓ Basic Example 2
    ✕ Basic Example 3 (3 ms)
    ✓ Influence Budget Redistribution
    ✓ Random Tiebreaker (1 ms)
    ✓ Random Tiebreaker with Defined Order (1 ms)
    ✓ Valid/Invalid/Under/Bullet Vote Counts (1 ms)

  ● Method of Equal Shares Tests › Basic Example 3

    expect(received).toBe(expected) // Object.is equality

    Expected: "Grace"
    Received: "Frank"

      81 |     expect(results.elected[0].name).toBe("Allison");
      82 |     expect(results.elected[1].name).toBe("Bill");
    > 83 |     expect(results.elected[2].name).toBe("Grace");
         |                                     ^
      84 |     expect(results.elected[3].name).toBe("Jack");
      85 |     expect(results.elected[4].name).toBe("Mona");
      86 |     expect(results.elected[5].name).toBe("Oscar");

      at Object.<anonymous> (src/Tabulators/MethodOfEqualShares.test.ts:83:37)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 6 passed, 7 total
Snapshots:   0 total
Time:        2.183 s, estimated 3 s

Not sure why this is failing...

 {
      candidates: [
        { index: 0, name: 'Allison', tieBreakOrder: 0 },
        { index: 1, name: 'Bill', tieBreakOrder: 1 },
        { index: 2, name: 'Carmen', tieBreakOrder: 2 },
        { index: 3, name: 'Doug', tieBreakOrder: 3 },
        { index: 4, name: 'Eve', tieBreakOrder: 4 },
        { index: 5, name: 'Frank', tieBreakOrder: 5 },
        { index: 6, name: 'Grace', tieBreakOrder: 6 },
        { index: 7, name: 'Hank', tieBreakOrder: 7 },
        { index: 8, name: 'Ivy', tieBreakOrder: 8 },
        { index: 9, name: 'Jack', tieBreakOrder: 9 },
        { index: 10, name: 'Karen', tieBreakOrder: 10 },
        { index: 11, name: 'Leo', tieBreakOrder: 11 },
        { index: 12, name: 'Mona', tieBreakOrder: 12 },
        { index: 13, name: 'Nina', tieBreakOrder: 13 },
        { index: 14, name: 'Oscar', tieBreakOrder: 14 },
        { index: 15, name: 'Paul', tieBreakOrder: 15 },
        { index: 16, name: 'Quinn', tieBreakOrder: 16 },
        { index: 17, name: 'Rita', tieBreakOrder: 17 },
        { index: 18, name: 'Sam', tieBreakOrder: 18 }
      ],
      totalScores: [
        { index: 0, score: 6 },
        { index: 1, score: 6 },
        { index: 2, score: 4 },
        { index: 3, score: 5 },
        { index: 4, score: 5 },
        { index: 5, score: 6 },
        { index: 6, score: 7 },
        { index: 7, score: 5 },
        { index: 8, score: 5 },
        { index: 9, score: 6 },
        { index: 10, score: 2 },
        { index: 11, score: 5 },
        { index: 12, score: 6 },
        { index: 13, score: 4 },
        { index: 14, score: 6 },
        { index: 15, score: 3 },
        { index: 16, score: NaN },
        { index: 17, score: NaN },
        { index: 18, score: NaN }
      ],
      nValidVotes: 10,
      nInvalidVotes: 0,
      nUnderVotes: 0,
      nBulletVotes: 0
    }

@mikefranze
Copy link
Collaborator

image


 FAIL  src/Tabulators/MethodOfEqualShares.test.ts
  Method of Equal Shares Tests
    ✓ Basic Example (3 ms)
    ✓ Basic Example 2
    ✕ Basic Example 3 (3 ms)
    ✓ Influence Budget Redistribution
    ✓ Random Tiebreaker (1 ms)
    ✓ Random Tiebreaker with Defined Order (1 ms)
    ✓ Valid/Invalid/Under/Bullet Vote Counts (1 ms)

  ● Method of Equal Shares Tests › Basic Example 3

    expect(received).toBe(expected) // Object.is equality

    Expected: "Grace"
    Received: "Frank"

      81 |     expect(results.elected[0].name).toBe("Allison");
      82 |     expect(results.elected[1].name).toBe("Bill");
    > 83 |     expect(results.elected[2].name).toBe("Grace");
         |                                     ^
      84 |     expect(results.elected[3].name).toBe("Jack");
      85 |     expect(results.elected[4].name).toBe("Mona");
      86 |     expect(results.elected[5].name).toBe("Oscar");

      at Object.<anonymous> (src/Tabulators/MethodOfEqualShares.test.ts:83:37)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 6 passed, 7 total
Snapshots:   0 total
Time:        2.183 s, estimated 3 s

Not sure why this is failing...

 {
      candidates: [
        { index: 0, name: 'Allison', tieBreakOrder: 0 },
        { index: 1, name: 'Bill', tieBreakOrder: 1 },
        { index: 2, name: 'Carmen', tieBreakOrder: 2 },
        { index: 3, name: 'Doug', tieBreakOrder: 3 },
        { index: 4, name: 'Eve', tieBreakOrder: 4 },
        { index: 5, name: 'Frank', tieBreakOrder: 5 },
        { index: 6, name: 'Grace', tieBreakOrder: 6 },
        { index: 7, name: 'Hank', tieBreakOrder: 7 },
        { index: 8, name: 'Ivy', tieBreakOrder: 8 },
        { index: 9, name: 'Jack', tieBreakOrder: 9 },
        { index: 10, name: 'Karen', tieBreakOrder: 10 },
        { index: 11, name: 'Leo', tieBreakOrder: 11 },
        { index: 12, name: 'Mona', tieBreakOrder: 12 },
        { index: 13, name: 'Nina', tieBreakOrder: 13 },
        { index: 14, name: 'Oscar', tieBreakOrder: 14 },
        { index: 15, name: 'Paul', tieBreakOrder: 15 },
        { index: 16, name: 'Quinn', tieBreakOrder: 16 },
        { index: 17, name: 'Rita', tieBreakOrder: 17 },
        { index: 18, name: 'Sam', tieBreakOrder: 18 }
      ],
      totalScores: [
        { index: 0, score: 6 },
        { index: 1, score: 6 },
        { index: 2, score: 4 },
        { index: 3, score: 5 },
        { index: 4, score: 5 },
        { index: 5, score: 6 },
        { index: 6, score: 7 },
        { index: 7, score: 5 },
        { index: 8, score: 5 },
        { index: 9, score: 6 },
        { index: 10, score: 2 },
        { index: 11, score: 5 },
        { index: 12, score: 6 },
        { index: 13, score: 4 },
        { index: 14, score: 6 },
        { index: 15, score: 3 },
        { index: 16, score: NaN },
        { index: 17, score: NaN },
        { index: 18, score: NaN }
      ],
      nValidVotes: 10,
      nInvalidVotes: 0,
      nUnderVotes: 0,
      nBulletVotes: 0
    }

@fsargent does this tool output any intermediate data that we can use to debug?

@mikefranze
Copy link
Collaborator

Could also check the box to use fractions instead of floats and check if we're hitting any tiebreaker scenarios.

@mikefranze
Copy link
Collaborator

Looks like a ties issue. In the tool you can click on the results at the bottom and get logs that includes a list of tied winner sets.

Your winners were: 0, 1, 5, 6, 9, 12

Tied winner sets:
0,1,5,6,12,14
0,1,6,9,12,14
0,1,5,6,8,12
0,1,5,6,9,12 <===
0,1,5,6,9,14
0,5,6,8,12,14
0,5,6,9,12,14
0,5,6,8,9,12
1,5,6,9,12,14

@fsargent fsargent force-pushed the feat_add_MethodOfEqualShares branch from 6ecc82e to 484ba7c Compare July 24, 2024 20:37
function shuffle<T>(array: T[]): T[] {
const shuffled = array.slice();
for (let i = shuffled.length - 1; i > 0; i--) {
const j = Math.floor(Math.random() * (i + 1));
Copy link
Collaborator

Choose a reason for hiding this comment

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

We shouldn't need a random generator here. The "randomTiebreakOrder" input to the main function is intended to determine how "random" ties are broken. This allows tests to be determinate and the server can generate a random order based on candidate UUIDs. Sorry that this wasn't documented.

Copy link
Collaborator Author

@fsargent fsargent Jul 25, 2024

Choose a reason for hiding this comment

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

@mikefranze Yeah, makes sense. I'm having issues because the existing tool (https://pref.tools/abcvoting/) doesn't actually describe what order the candidates are elected, so it's hard for me to write tests without me just 'guessing' who should win, or trusting my possibly faulty code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants