feat: add round-trace APIs for IRV, Coombs, and Baldwin/Total Vote Runoff - #71
feat: add round-trace APIs for IRV, Coombs, and Baldwin/Total Vote Runoff#71devin-ai-integration[bot] wants to merge 3 commits into
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
PR Summary
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #71 +/- ##
==========================================
+ Coverage 96.37% 97.23% +0.85%
==========================================
Files 17 18 +1
Lines 496 686 +190
==========================================
+ Hits 478 667 +189
- Misses 18 19 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
📝 WalkthroughWalkthroughThe change adds round-trace APIs for Baldwin, Total Vote Runoff, Coombs, and IRV. It adds immutable result records, configurable stopping, transfer details, validation, public exports, and tests for counting behavior and tie handling. ChangesVoting method round traces
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
tests/test_coombs_rounds.py (1)
59-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: also cover the non-integer
stop_atrejection.
_validate_stop_atraisesTypeErrorforbooland non-integer input. This suite exercises only theValueErrorrange check.💚 Proposed test addition
+@pytest.mark.parametrize('stop_at', [True, 1.0, '1']) +def test_coombs_rounds_rejects_non_integer_stop_counts(stop_at): + """Non-integer survivor counts must not be coerced silently.""" + election = np.array([ + [0, 1, 2], + [1, 2, 0], + ]) + + with pytest.raises(TypeError, match='stop_at must be an integer'): + coombs_rounds(election, stop_at=stop_at)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_coombs_rounds.py` around lines 59 - 68, Extend test_coombs_rounds_rejects_out_of_range_stop_counts or add a focused parametrized test covering boolean and non-integer stop_at values, asserting coombs_rounds raises TypeError with the expected validation message from _validate_stop_at.elsim/methods/coombs.py (2)
115-121: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueOptional: drop one redundant top-choice tally per traced round.
Lines 119-121 tally the top choices after the elimination. The next iteration tallies the same cursors again at Lines 78-80, and the post-loop call at Line 140 repeats it for the last round. Each tally is an O(n_voters) Python loop in
_tally_at_rank_idx, so the traced path costs about twice the winner-only path.You can keep
first_tallies_afteras a view of the next round'sfirst_tallies_beforeinstead, or record the transfer deltas and reuse the next iteration's tally. This is a performance nit only; the recorded values are correct today.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elsim/methods/coombs.py` around lines 115 - 121, In the record_rounds branch of the Coombs election flow, remove the redundant _tally_at_rank_idx call after elimination. Preserve first_tallies_after by reusing the next iteration’s first_tallies_before view or equivalent transfer-delta data, while retaining correct final-round recording without repeating the same O(n_voters) tally.
23-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider hardening the frozen records against mutation and comparison errors.
frozen=Trueprevents attribute rebinding only. The recorded arrays stay writable, so a caller can alterfirst_tallies_beforein place after the count. Also, the generated__eq__compares ndarray fields, soround_a == round_braisesValueError: The truth value of an array with more than one element is ambiguous.If you want the records to behave as immutable values, mark the arrays read-only when you build them and disable the generated equality.
♻️ Proposed refactor
-@dataclass(frozen=True) +@dataclass(frozen=True, eq=False) class CoombsRound:-@dataclass(frozen=True) +@dataclass(frozen=True, eq=False) class CoombsResult:Then set
arr.flags.writeable = Falseon each array before you store it in a record.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elsim/methods/coombs.py` around lines 23 - 58, Harden the frozen CoombsRound and CoombsResult records by disabling generated dataclass equality and marking every stored ndarray read-only before construction, including tally, voter, candidate, choice, and transfer arrays. Update the record-building paths around these classes so callers cannot mutate the recorded arrays and equality comparisons do not invoke ambiguous ndarray comparison.tests/test_baldwin.py (1)
81-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd negative-path coverage for
stop_atvalidation.The suite tests
stop_at=2for early termination but does not exercise_validate_stop_at's error paths (non-integerstop_at, orstop_atoutside[1, n_cands]) throughbaldwin_rounds/total_vote_runoff_rounds. Add a test assertingTypeError/ValueErrorfor invalidstop_atvalues to guard this shared validation contract at its Baldwin/TVR entry points.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_baldwin.py` around lines 81 - 88, Add negative-path tests covering the shared stop_at validation used by baldwin_rounds and total_vote_runoff_rounds. Exercise non-integer values and integers outside the inclusive range [1, n_cands], asserting TypeError or ValueError as appropriate, while preserving the existing valid stop_at=2 test.elsim/methods/baldwin.py (2)
24-66: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFrozen dataclasses with ndarray fields break default equality.
BaldwinRoundandBaldwinResultare@dataclass(frozen=True)withnp.ndarrayfields. The default generated__eq__compares fields as a tuple, and tuple equality raisesValueError: The truth value of an array with more than one element is ambiguouswhen an ndarray field has more than one element. Any caller who compares twoBaldwinRound/BaldwinResultinstances with==(a natural pattern for these trace record types) hits this crash instead of a boolean result.Set
eq=Falseand, if needed, add a custom__eq__usingnp.array_equalper field, or document that these types are not comparable with==.Separately,
frozen=Trueblocks field reassignment but does not stop in-place mutation of the array contents (result.rounds[0].borda_before[0] = 999still succeeds). If true immutability matters here, call.setflags(write=False)on the arrays before storing them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elsim/methods/baldwin.py` around lines 24 - 66, Update BaldwinRound and BaldwinResult to avoid generated ndarray equality raising ambiguous truth-value errors: set eq=False and implement explicit __eq__ methods that compare ndarray fields with np.array_equal and scalar/tuple fields appropriately. If these trace records are intended to be immutable, also mark stored ndarray fields read-only before construction while preserving their existing frozen dataclass behavior.
68-93: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftVectorize
_borda_scoresfor repeated simulation use.
_borda_scoresrecomputes Borda scores with a pure-Python nested loop over every ballot and every candidate, and it runs unconditionally on every round of_run_baldwin(not only whenrecord_roundsis set). This gives O(n_voters * n_cands) Python-level work per round, and O(n_cands) rounds, for O(n_voters * n_cands²) total per count. The existingborda()inborda.pyachieves the equivalent full-candidate-set computation with vectorizednp.bincountcalls per column.Since this library appears to simulate elections repeatedly, this cost compounds across many Baldwin/TVR counts. Consider a vectorized approach that masks eliminated candidates and computes rank positions with NumPy operations (e.g.,
argsort/searchsortedtricks) instead of a per-ballot Python loop.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elsim/methods/baldwin.py` around lines 68 - 93, The `_borda_scores` implementation performs nested Python loops for every ballot and candidate, making repeated Baldwin rounds unnecessarily slow. Replace the loop-based scoring in `_borda_scores` with NumPy-vectorized operations that apply `eliminated_mask`, derive active-candidate rank positions, and aggregate points into the candidate score array while preserving the existing one-based Borda scores and zero scores for eliminated candidates.elsim/methods/irv.py (1)
165-168: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that majority detection is not rechecked once
stop_atis already satisfied.The text states: "Majority winners still end the count immediately" (Lines 167). This reads as an unconditional guarantee, but it only applies while the
whileloop keeps running (that is, whileactive_candidates > stop_at).test_irv_rounds_can_stop_with_two_candidatesdemonstrates the opposite: after the loop exits,final_tallies=[3,2,0]represents an outright majority (3 of 5), yetresult.winnerisNone.Reword this sentence to state that majority is checked only for rounds that would otherwise continue past
stop_at, not for the state already reached whenstop_atis satisfied (whether via round elimination or the initial zero-vote exclusion above).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@elsim/methods/irv.py` around lines 165 - 168, Update the docstring for the stop_at parameter to clarify that majority detection occurs only during loop iterations while active_candidates exceeds stop_at. Replace the phrase "Majority winners still end the count immediately" with wording that specifies majority is checked only for rounds that would continue past stop_at, and that once the loop exits due to stop_at being satisfied (whether by elimination or initial state), no further majority checking occurs for the reached state. This ensures the documentation accurately reflects that a majority candidate in the final state when stop_at is satisfied will not be returned as a winner.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@elsim/methods/coombs.py`:
- Around line 136-138: Update coombs_rounds so that when the elimination loop
ends because stop_at is reached, including stop_at == n_cands, it evaluates
final_tallies for a majority winner before falling back to the single active
candidate logic. Preserve the documented contract that any candidate holding a
majority among the final tallies is reported in winner.
In `@elsim/methods/irv.py`:
- Around line 77-85: The initial batch elimination of zero-vote candidates in
the initially_eliminated block violates the stop_at API contract by
unconditionally removing all zero-vote candidates before the while loop checks
the floor. Add a guard condition that calculates how many candidates from
initially_eliminated can be safely removed without dropping the active candidate
count below stop_at (by subtracting stop_at and the count of already-eliminated
candidates from the total candidate count), then only apply the elimination mask
to that safe subset. This preserves the stop_at requirement while still
eliminating zero-vote candidates when permitted.
---
Nitpick comments:
In `@elsim/methods/baldwin.py`:
- Around line 24-66: Update BaldwinRound and BaldwinResult to avoid generated
ndarray equality raising ambiguous truth-value errors: set eq=False and
implement explicit __eq__ methods that compare ndarray fields with
np.array_equal and scalar/tuple fields appropriately. If these trace records are
intended to be immutable, also mark stored ndarray fields read-only before
construction while preserving their existing frozen dataclass behavior.
- Around line 68-93: The `_borda_scores` implementation performs nested Python
loops for every ballot and candidate, making repeated Baldwin rounds
unnecessarily slow. Replace the loop-based scoring in `_borda_scores` with
NumPy-vectorized operations that apply `eliminated_mask`, derive
active-candidate rank positions, and aggregate points into the candidate score
array while preserving the existing one-based Borda scores and zero scores for
eliminated candidates.
In `@elsim/methods/coombs.py`:
- Around line 115-121: In the record_rounds branch of the Coombs election flow,
remove the redundant _tally_at_rank_idx call after elimination. Preserve
first_tallies_after by reusing the next iteration’s first_tallies_before view or
equivalent transfer-delta data, while retaining correct final-round recording
without repeating the same O(n_voters) tally.
- Around line 23-58: Harden the frozen CoombsRound and CoombsResult records by
disabling generated dataclass equality and marking every stored ndarray
read-only before construction, including tally, voter, candidate, choice, and
transfer arrays. Update the record-building paths around these classes so
callers cannot mutate the recorded arrays and equality comparisons do not invoke
ambiguous ndarray comparison.
In `@elsim/methods/irv.py`:
- Around line 165-168: Update the docstring for the stop_at parameter to clarify
that majority detection occurs only during loop iterations while
active_candidates exceeds stop_at. Replace the phrase "Majority winners still
end the count immediately" with wording that specifies majority is checked only
for rounds that would continue past stop_at, and that once the loop exits due to
stop_at being satisfied (whether by elimination or initial state), no further
majority checking occurs for the reached state. This ensures the documentation
accurately reflects that a majority candidate in the final state when stop_at is
satisfied will not be returned as a winner.
In `@tests/test_baldwin.py`:
- Around line 81-88: Add negative-path tests covering the shared stop_at
validation used by baldwin_rounds and total_vote_runoff_rounds. Exercise
non-integer values and integers outside the inclusive range [1, n_cands],
asserting TypeError or ValueError as appropriate, while preserving the existing
valid stop_at=2 test.
In `@tests/test_coombs_rounds.py`:
- Around line 59-68: Extend test_coombs_rounds_rejects_out_of_range_stop_counts
or add a focused parametrized test covering boolean and non-integer stop_at
values, asserting coombs_rounds raises TypeError with the expected validation
message from _validate_stop_at.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: dd594f34-3c2b-4cd6-8082-3aedb5cb9b2e
📒 Files selected for processing (8)
elsim/methods/__init__.pyelsim/methods/_common.pyelsim/methods/baldwin.pyelsim/methods/coombs.pyelsim/methods/irv.pytests/test_baldwin.pytests/test_coombs_rounds.pytests/test_irv_rounds.py
| active_candidates = np.flatnonzero(~eliminated_mask) | ||
| if winner is None and active_candidates.size == 1: | ||
| winner = int(active_candidates[0]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
winner stays None when stop_at ends the count on a majority holder.
The majority test at Line 84 runs only inside the loop body. If the loop exits because the active count already equals stop_at, no majority test runs. A candidate who holds a majority among the survivors is then not reported.
The new test at tests/test_coombs_rounds.py:41-45 shows this: final_tallies is [2, 0, 3], so candidate 2 holds 3 of 5 first choices, but result.winner is None. The coombs_rounds docstring at Lines 166-167 states that majority winners still end the count immediately, so the documented contract and the behavior disagree. The same applies for stop_at == n_cands, where the loop body never runs.
Decide which contract you want. Either set the winner from the final tallies, or state in the docstring that winner is reported only for a majority reached during an elimination round or for a single survivor.
🔧 Option A: report a majority winner from the final tallies
active_candidates = np.flatnonzero(~eliminated_mask)
- if winner is None and active_candidates.size == 1:
- winner = int(active_candidates[0])
-
_tally_at_rank_idx(cand_top_tallies, election, voter_top_rank_idx)
+ if winner is None:
+ if active_candidates.size == 1:
+ winner = int(active_candidates[0])
+ else:
+ final_tallies_list = cand_top_tallies.tolist()
+ max_final_tally = max(final_tallies_list)
+ if max_final_tally > n_voters / 2:
+ winner = final_tallies_list.index(max_final_tally)
+
final_choices = election[📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| active_candidates = np.flatnonzero(~eliminated_mask) | |
| if winner is None and active_candidates.size == 1: | |
| winner = int(active_candidates[0]) | |
| active_candidates = np.flatnonzero(~eliminated_mask) | |
| _tally_at_rank_idx(cand_top_tallies, election, voter_top_rank_idx) | |
| if winner is None: | |
| if active_candidates.size == 1: | |
| winner = int(active_candidates[0]) | |
| else: | |
| final_tallies_list = cand_top_tallies.tolist() | |
| max_final_tally = max(final_tallies_list) | |
| if max_final_tally > n_voters / 2: | |
| winner = final_tallies_list.index(max_final_tally) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@elsim/methods/coombs.py` around lines 136 - 138, Update coombs_rounds so that
when the elimination loop ends because stop_at is reached, including stop_at ==
n_cands, it evaluates final_tallies for a majority winner before falling back to
the single active candidate logic. Preserve the documented contract that any
candidate holding a majority among the final tallies is reported in winner.
| # A candidate with no first choices cannot gain any transfers before | ||
| # another candidate is eliminated. Excluding all such candidates together | ||
| # preserves the historical IRV behavior without inventing an arbitrary | ||
| # order among candidates tied at zero. | ||
| _tally_at_rank_idx(cand_tallies, election, voter_top_rank_idx) | ||
| initially_eliminated = np.flatnonzero(cand_tallies == 0) | ||
| eliminated_mask[initially_eliminated] = True | ||
| if initially_eliminated.size: | ||
| _inc_rank_idx(election, voter_top_rank_idx, eliminated_mask) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
python3 - <<'EOF'
import numpy as np
from elsim.methods.irv import irv_rounds
# 5 candidates; c, d, e get zero first-choice votes.
election = np.array([
[0, 1, 2, 3, 4],
[0, 2, 1, 3, 4],
[1, 0, 2, 3, 4],
])
result = irv_rounds(election, stop_at=4)
print("active_candidates:", result.active_candidates)
print("winner:", result.winner)
print("initially_eliminated:", result.initially_eliminated)
EOFRepository: endolith/elsim
Length of output: 270
🏁 Script executed:
cat -n elsim/methods/irv.py | head -100Repository: endolith/elsim
Length of output: 4159
🏁 Script executed:
cat -n elsim/methods/irv.py | sed -n '65,200p'Repository: endolith/elsim
Length of output: 6168
🏁 Script executed:
cat -n elsim/methods/_common.py | grep -A 10 "_validate_stop_at"Repository: endolith/elsim
Length of output: 649
🏁 Script executed:
find . -name "*.py" -type f -exec grep -l "test_irv_rounds_can_stop_with_two_candidates" {} \;Repository: endolith/elsim
Length of output: 179
🏁 Script executed:
grep -A 20 "test_irv_rounds_can_stop_with_two_candidates" ./tests/test_irv_rounds.pyRepository: endolith/elsim
Length of output: 825
🏁 Script executed:
python3 - <<'PY'
# Simulate the logic without needing numpy
# 5 candidates: 0, 1, 2, 3, 4
# First-choice votes: only candidates 0 and 1 get votes
# Candidates 2, 3, 4 get zero votes
n_cands = 5
stop_at = 4
# Simulate cand_tallies
cand_tallies = [1, 1, 0, 0, 0] # candidates 0,1 have votes; 2,3,4 have zero
# Initially eliminated: candidates with zero votes
initially_eliminated = [i for i, tally in enumerate(cand_tallies) if tally == 0]
print(f"Initially eliminated: {initially_eliminated}")
print(f"Count: {len(initially_eliminated)}")
# Active candidates after initial elimination
active_after_initial = [i for i in range(n_cands) if i not in initially_eliminated]
print(f"Active candidates after initial elimination: {active_after_initial}")
print(f"Active count: {len(active_after_initial)}")
# Check while loop condition
active_count = len(active_after_initial)
while_condition = active_count > stop_at
print(f"\nWhile loop condition: {active_count} > {stop_at} = {while_condition}")
print(f"Loop will run: {while_condition}")
print(f"\nResult: {len(active_after_initial)} active candidates, but stop_at={stop_at}")
print(f"UNDERSHOOT: {len(active_after_initial)} < {stop_at}")
PYRepository: endolith/elsim
Length of output: 381
Initial batch elimination of zero-vote candidates can drop active candidates below the stop_at floor, violating the API contract.
The docstring for stop_at states: "Stop before eliminating below this number of active candidates" (line 166). The unconditional batch elimination at lines 77-85 runs before the while loop checks stop_at at line 87, so it can remove multiple candidates simultaneously and land below the requested floor.
Example: 5 candidates total; only candidates 0 and 1 receive first-choice votes; candidates 2, 3, and 4 receive zero votes. Calling irv_rounds(election, stop_at=4) expects at least 4 active candidates to remain, but the initially_eliminated batch removes all three zero-vote candidates in a single step, leaving only 2 active candidates. The caller cannot predict this in advance because the zero-vote count is only known after tallying.
This breaks the "Animation callers need a complete trace through" requirement that stop_at supports (see docstring and test_irv_rounds_can_stop_with_two_candidates).
To fix: Either gate the initial batch elimination to never drop the active candidate count below stop_at, or explicitly document that initially_eliminated can independently undershoot the stop_at parameter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@elsim/methods/irv.py` around lines 77 - 85, The initial batch elimination of
zero-vote candidates in the initially_eliminated block violates the stop_at API
contract by unconditionally removing all zero-vote candidates before the while
loop checks the floor. Add a guard condition that calculates how many candidates
from initially_eliminated can be safely removed without dropping the active
candidate count below stop_at (by subtracting stop_at and the count of
already-eliminated candidates from the total candidate count), then only apply
the elimination mask to that safe subset. This preserves the stop_at requirement
while still eliminating zero-vote candidates when permitted.
bf15c21 to
68a3d3f
Compare
Animation and analysis callers need the state transitions from each IRV elimination, but the existing API returns only a winner. Reimplement the winner path through a shared count that can also return typed round and result records, including first-choice tallies, affected voter indices, transfer destinations, active candidates, and final state. Candidates with no initial first-choice votes remain batch-eliminated to preserve existing winner semantics. Their IDs are now explicit in the result rather than silently missing from the trace. A stop_at option supports final-two visualizations while majority detection continues to end an ordinary IRV count early. Regression tests verify exact transfers, zero-vote exclusions, final-two stopping, tie behavior, validation, and parity with the existing irv() winner API. Co-Authored-By: Endolith <endolith@gmail.com>
Coombs counts previously exposed only the winner, so callers could not inspect why a candidate was eliminated or animate the resulting first-choice transfers. Add typed result and round records containing the decisive last-place totals, first-choice tallies before and after elimination, affected voters, and their transfer destinations. The winner API and trace API now share one count implementation, including majority short-circuiting and tie behavior. A stop_at option uses the same validation as IRV so analysis tools can request a final-two state without duplicating counting logic. Tests cover the exact last-place elimination and transfers, final-two stopping, unresolved ties, invalid arguments, and winner parity with coombs(). Co-Authored-By: Endolith <endolith@gmail.com>
Add baldwin_rounds and a total_vote_runoff alias that record each Borda elimination for the collapse animations, including Borda scores before and after elimination, first-choice tallies and transfers, and each voter's active candidates ranked above the loser. Both names run the same count: iteratively eliminate the lowest-Borda candidate while recomputing scores among the remaining candidates, and stop when one remains or a candidate has a first-choice majority. A majority candidate is the Condorcet winner and necessarily wins. Preserve the one-based Borda transition model used by the animation regression tests. Co-authored-by: Endolith <endolith@gmail.com>
68a3d3f to
f3518f3
Compare
1 participant
Summary
Background. The
collapse_2dbranch builds 2D "center-squeeze" animations that need the complete per-round state of an elimination count (who was eliminated, the tallies before/after, and exactly which voters transferred). Todayirv/coombsonly return a winner, so that branch reimplemented the elimination logic inside plotting code.Problem. Reconstructing counts in plotting code was error-prone: it dropped IRV's initial zero-vote exclusions and modeled Borda-score transitions in the wrong direction.
What this PR changes. It factors each count into a shared
_run_*helper and exposes authoritative round traces as frozen dataclasses, without changing the existingirv/coombswinner APIs. This is the foundation PR of the collapse_2d rewrite; the spatial-search and animation example scripts land as stacked PRs on top.New public API:
Key design points a reviewer can't infer from the diff alone:
IRVResult.initially_eliminated, so a consumer can reconstruct the full candidate state (the old ad-hoc traces silently lost them).total_vote_runoff*simply delegates to the Baldwin implementation.baldwinhere is the repo's first standalone Baldwin/TVR winner function.BaldwinRoundrecordsborda_before/borda_after(one-based: last active candidate gets 1 point) plushigher_ranked_candidates— per voter, the active candidates ranked above the loser, each of which loses exactly one point as the field shrinks. This lets a consumer animate the retally without re-deriving it.stop_atlets a caller stop the count early (e.g.stop_at=2to freeze at the final two) while still ending immediately on a majority. Validated by_validate_stop_at.irv,coombs,baldwin,total_vote_runoff) now delegate to the shared_run_*helpers withrecord_rounds=False, so winner behavior is unchanged and covered by existing tests.Each
*Rounddataclass also recordstransferred_voters/transferred_to(the voters whose first choice was the loser and where their vote went), enabling exact transfer animations.Tests
New
tests/test_irv_rounds.py,tests/test_coombs_rounds.py,tests/test_baldwin.pycover: exact tallies and transfer destinations per round; IRV's explicit zero-vote exclusions;stop_atearly stopping and argument validation; winner parity with the existingirv/coombsAPIs; Baldwin and TVR being the same count (both stop once a transfer creates a first-choice majority); and reconstruction of every Borda-score transition. Full suite:240 passed; full pre-commit / Ruff gate clean.Related work
First PR in the collapse_2d rewrite stack. The 2D spatial-search and animation/rendering example scripts (which consume these traces) are stacked on top in #72.
Link to Devin session: https://app.devin.ai/sessions/d2d2ac0338354d7ab08c4468ee3e1680
Requested by: @endolith