feat(examples): rebuild 2D center-squeeze collapse animations on the round-trace API - #72
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:
|
934571e to
3976b9e
Compare
You spent $13.41 of my free trial on this single PR and … actively made it worse. Not going to be using Devin going forward lol. That's like a whole month's worth of spending on Open Interpreter Classic with DeepSeek V4 Flash. (aside) |
📝 WalkthroughWalkthroughAdded shared 2D election visualization utilities and standalone IRV, Baldwin, and combined election animation scripts. Added election-search validation, GIF generation, regression tests, documentation, and output-directory exclusion. Changes2D center-squeeze animations
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant find_both_election
participant election_to_traces
participant run_irv_animation
participant run_tvr_animation
find_both_election->>election_to_traces: sample electorate and generate IRV/Baldwin traces
election_to_traces-->>find_both_election: return qualifying election data
find_both_election->>run_irv_animation: render IRV frames and GIF
find_both_election->>run_tvr_animation: render Baldwin frames and GIF
🚥 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: 3
🧹 Nitpick comments (10)
examples/collapse_finder_2d_tvr.py (6)
1-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAlign the module name with the method it actually runs.
The module is named
collapse_finder_2d_tvrand the docstring describes Total Vote Runoff, but the code only callsbaldwin_rounds.baldwin_roundsexplicitly documents that it does not stop on a first-choice majority, which is the one behavior that separates Baldwin from Total Vote Runoff. A reader cannot tell from the file name which trace is rendered.Consider stating in the docstring that this example renders the Baldwin count, and that Total Vote Runoff is mentioned only for context.
🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 1 - 6, Update the module docstring for the example that calls baldwin_rounds to state explicitly that it renders the Baldwin count; identify Total Vote Runoff only as contextual comparison, without describing the rendered trace as TVR.
304-308: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRaise a
ValueErrorinstead ofAssertionErrorfor the trace mismatch.The check validates data that arrives from
baldwin_rounds, not an internal invariant of this function.AssertionErrorsuggests a failedassertstatement and reads as a defect in the replay code. AValueErrorthat names the round makes the failure easier to diagnose.♻️ Suggested change
if step == frames_per_transfer - 1: if not np.array_equal(running_borda, round_.borda_after): - raise AssertionError('Baldwin Borda transition did not match trace.') + raise ValueError( + f'Round {round_index}: replayed Borda scores ' + f'{running_borda.tolist()} do not match the recorded ' + f'scores {round_.borda_after.tolist()}.' + )🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 304 - 308, In the end-of-transfer validation within the replay loop, replace the AssertionError raised for a running_borda versus round_.borda_after mismatch with ValueError. Include the round identifier in the error message so invalid data from baldwin_rounds is diagnosable, while preserving the existing successful transition and running_choices updates.
106-109: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the mask loop with a vectorized expression.
np.maximumalready clamps negative values to 0, then the loop zeroes the same entries again forborda_scores[candidate] <= 0. One masked assignment covers both cases.♻️ Suggested simplification
- bar_segments = np.maximum(average_scores, 0) - for candidate in range(n_cands): - if borda_scores[candidate] <= 0: - bar_segments[candidate] = 0 + bar_segments = np.where(np.asarray(borda_scores) > 0, average_scores, 0.0)🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 106 - 109, In the bar-segment preparation around bar_segments, remove the per-candidate loop and apply a vectorized mask using borda_scores to zero entries where scores are non-positive. Preserve np.maximum’s clamping of negative average_scores while combining the mask into a single array operation.
365-388: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReport search progress for the 100,000-trial default.
Each trial samples 5000 voters, builds a 5000×9 utility matrix, derives rankings and a pairwise matrix, and runs Baldwin. With
max_trials = 100_000and no progress output, a hard search looks like a hang, and theRuntimeErrorarrives only at the end.Consider printing every N trials, or accepting a time budget in
find_center_convergent_election.🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 365 - 388, Update the main search flow around find_center_convergent_election to report progress during the 100,000-trial search, using periodic trial updates or the function’s existing progress mechanism if available. Ensure users receive visible status before completion or the final RuntimeError, without changing the search result behavior.
350-361: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winThe GIF frame list is rebuilt from the filesystem, and the test cannot detect a mismatch.
run_tvr_animationtracks onedurationsentry per rendered frame, but it recovers the images by globbing[0-9][0-9][0-9][0-9].png. The two lists are therefore correlated only by convention, and the smoke test checks file existence only, so any divergence stays silent.
examples/collapse_finder_2d_tvr.py#L350-L361: collect each frame path as you render it, then buildimagesfrom that list instead of re-globbing, and open the images inside acontextlib.ExitStackso the handles close whensaveraises.tests/test_collapse_finder_2d_tvr.py#L113-L123: assert that the saved GIFn_framesequals the number of PNG frames written, so a durations-to-images mismatch fails the test.🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 350 - 361, In examples/collapse_finder_2d_tvr.py lines 350-361, update run_tvr_animation to retain each rendered frame path during rendering, build images from that retained list instead of filesystem globbing, and open them through contextlib.ExitStack so handles close even if GIF saving raises. In tests/test_collapse_finder_2d_tvr.py lines 113-123, assert the saved GIF’s n_frames matches the number of written PNG frames.
61-77: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a
random_stateparameter to thread through the election search.The function calls
normal_electoratewithoutrandom_state, so the election sampling is nondeterministic. The animation steprun_tvr_animationacceptsseedand is reproducible, but the selected electorate is not. A user cannot recreate the same electorate from a saved GIF.♻️ Suggested change
-def find_center_convergent_election(n_voters, n_cands, max_trials, disp=1.0): +def find_center_convergent_election( + n_voters, + n_cands, + max_trials, + disp=1.0, + random_state=None, +): """Sample elections whose center candidate is the Condorcet winner.""" + rng = np.random.default_rng(random_state) for trial in range(1, max_trials + 1): voters, candidates = normal_electorate( n_voters, n_cands, dims=2, disp=disp, + random_state=rng, )🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 61 - 77, Add a random_state parameter to find_center_convergent_election and pass it to normal_electorate on every sampling attempt, preserving the value across the search so the selected electorate is reproducible and can align with the animation seed.tests/test_collapse_finder_2d_tvr.py (2)
62-70: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd a case with no Condorcet winner.
The two cases here cover the winner-mismatch branch. The
condorcet_winner is Nonebranch invalidate_condorcet_winnerstays uncovered. A three-voter Condorcet cycle, for example ballots[0,1,2],[1,2,0],[2,0,1], makescondorcet_from_matrixreturnNoneand should makesimulate_tvr_roundsreturnNoneeven though Baldwin still produces a winner.🤖 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_collapse_finder_2d_tvr.py` around lines 62 - 70, Add a test case alongside test_center_validator_accepts_verified_condorcet_winner_and_rejects_other using the three-voter cyclic ballots [0,1,2], [1,2,0], and [2,0,1]. Assert condorcet(election) is None, confirm Baldwin still yields a winner, and verify simulate_tvr_rounds returns None to cover the condorcet_winner is None branch in validate_condorcet_winner.
43-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert against the production replay rule, not a copy of it.
This loop reimplements the exact decrement rule from
run_tvr_animationat Lines 297-300. The test therefore verifies a duplicate of the logic. If someone changes the rule in the animation and not here, the test still passes while the rendered Borda bars drift from the trace.Extract the rule into a small helper in
examples/collapse_finder_2d_tvr.py, for exampleapply_elimination_to_borda(borda, higher_ranked, loser, n_active), then call that helper from both the animation loop and this test.🤖 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_collapse_finder_2d_tvr.py` around lines 43 - 49, Extract the shared Borda decrement logic from run_tvr_animation into an apply_elimination_to_borda helper in examples/collapse_finder_2d_tvr.py, accepting borda, higher_ranked, loser, and n_active. Replace the animation’s inline updates and the test loop’s duplicated calculations with calls to this helper so both paths use the production rule.tests/test_collapse_2d_shared.py (2)
91-108: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
eliminatedpath.This test only calls
create_frame_scaffoldwith the defaulteliminated=None. The elimination handling (gray substitution inactive_colors, exclusion from the scatter markers/annotations, and exclusion from the Voronoi diagram at Lines 316-322 and 364-369 ofexamples/collapse_2d_shared.py) is core to how both the IRV and Baldwin renderers depict eliminated candidates. Add a test that passes a non-emptyeliminatedset and asserts the corresponding entry inactive_colorsis grayed out.🤖 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_collapse_2d_shared.py` around lines 91 - 108, The test around create_frame_scaffold should also cover a non-empty eliminated set. Add a scaffold invocation with an eliminated candidate, capture active_colors from the return value, and assert that the eliminated candidate’s color is replaced with the expected gray value while non-eliminated colors remain unchanged.
44-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the ≥3-point Voronoi branch; fix the docstring direction.
Two points here share the same x-coordinate, so the segment connecting them is vertical and the resulting perpendicular bisector is horizontal, not vertical as the docstring states. This is a wording nit only; the test itself is correct.
More importantly, this test only exercises the 2-point special case in
voronoi_plot_2d_axes. Thelen(points) >= 3branch (Lines 225-263 ofexamples/collapse_2d_shared.py) contains the most complex logic in the module: finite/infinite ridge classification, tangent/normal computation, and the sign-based outward direction for unbounded ridges. Add a test with 3 or more points, including at least one unbounded ridge, to guard this path against regressions.✅ Example additional test
def test_voronoi_extends_unbounded_ridges_for_three_points(): """The ≥3-point branch should draw both finite and infinite ridge segments.""" fig, axis = plt.subplots() axis.set_xlim(-3, 3) axis.set_ylim(-3, 3) voronoi_plot_2d_axes( axis, np.array([[-1.0, 0.0], [1.0, 0.0], [0.0, 1.0]]), ) assert len(axis.collections) >= 1 plt.close(fig)🤖 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_collapse_2d_shared.py` around lines 44 - 56, Correct the two-point test docstring to describe a horizontal perpendicular bisector, then add coverage for the len(points) >= 3 path in voronoi_plot_2d_axes using at least three points that produce an unbounded ridge; assert that plotting creates the expected rendered geometry and close the figure consistently with the existing test.
🤖 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 `@examples/collapse_2d_shared.py`:
- Around line 163-196: Align the y-axis terminology in plot_favorability_bar
with the panel title by replacing the “Mean utility” wording in ax.set_ylabel
with “Average favorability” while preserving the percentage unit.
In `@examples/collapse_finder_2d_tvr.py`:
- Around line 294-303: In the round-processing loop around `transferred_voters`,
precompute a voter-to-target lookup once before iterating through voters, then
retrieve each voter's target directly instead of calling `np.flatnonzero` per
voter. Preserve the existing update to `running_choices` only for voters present
in the transfer map, and define a deterministic value for duplicate voter
entries without relying on `transferred[0]`.
- Line 16: Add matplotlib to the test extras dependency list in pyproject.toml,
while preserving the existing examples extras declaration, so
tests/test_collapse_finder_2d_tvr.py can import matplotlib in a clean test-only
environment.
---
Nitpick comments:
In `@examples/collapse_finder_2d_tvr.py`:
- Around line 1-6: Update the module docstring for the example that calls
baldwin_rounds to state explicitly that it renders the Baldwin count; identify
Total Vote Runoff only as contextual comparison, without describing the rendered
trace as TVR.
- Around line 304-308: In the end-of-transfer validation within the replay loop,
replace the AssertionError raised for a running_borda versus round_.borda_after
mismatch with ValueError. Include the round identifier in the error message so
invalid data from baldwin_rounds is diagnosable, while preserving the existing
successful transition and running_choices updates.
- Around line 106-109: In the bar-segment preparation around bar_segments,
remove the per-candidate loop and apply a vectorized mask using borda_scores to
zero entries where scores are non-positive. Preserve np.maximum’s clamping of
negative average_scores while combining the mask into a single array operation.
- Around line 365-388: Update the main search flow around
find_center_convergent_election to report progress during the 100,000-trial
search, using periodic trial updates or the function’s existing progress
mechanism if available. Ensure users receive visible status before completion or
the final RuntimeError, without changing the search result behavior.
- Around line 350-361: In examples/collapse_finder_2d_tvr.py lines 350-361,
update run_tvr_animation to retain each rendered frame path during rendering,
build images from that retained list instead of filesystem globbing, and open
them through contextlib.ExitStack so handles close even if GIF saving raises. In
tests/test_collapse_finder_2d_tvr.py lines 113-123, assert the saved GIF’s
n_frames matches the number of written PNG frames.
- Around line 61-77: Add a random_state parameter to
find_center_convergent_election and pass it to normal_electorate on every
sampling attempt, preserving the value across the search so the selected
electorate is reproducible and can align with the animation seed.
In `@tests/test_collapse_2d_shared.py`:
- Around line 91-108: The test around create_frame_scaffold should also cover a
non-empty eliminated set. Add a scaffold invocation with an eliminated
candidate, capture active_colors from the return value, and assert that the
eliminated candidate’s color is replaced with the expected gray value while
non-eliminated colors remain unchanged.
- Around line 44-56: Correct the two-point test docstring to describe a
horizontal perpendicular bisector, then add coverage for the len(points) >= 3
path in voronoi_plot_2d_axes using at least three points that produce an
unbounded ridge; assert that plotting creates the expected rendered geometry and
close the figure consistently with the existing test.
In `@tests/test_collapse_finder_2d_tvr.py`:
- Around line 62-70: Add a test case alongside
test_center_validator_accepts_verified_condorcet_winner_and_rejects_other using
the three-voter cyclic ballots [0,1,2], [1,2,0], and [2,0,1]. Assert
condorcet(election) is None, confirm Baldwin still yields a winner, and verify
simulate_tvr_rounds returns None to cover the condorcet_winner is None branch in
validate_condorcet_winner.
- Around line 43-49: Extract the shared Borda decrement logic from
run_tvr_animation into an apply_elimination_to_borda helper in
examples/collapse_finder_2d_tvr.py, accepting borda, higher_ranked, loser, and
n_active. Replace the animation’s inline updates and the test loop’s duplicated
calculations with calls to this helper so both paths use the production rule.
🪄 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: 1e93fa0f-4377-4d40-a142-82a4dd6a876a
📒 Files selected for processing (10)
.gitignoreexamples/README.mdexamples/collapse_2d_shared.pyexamples/collapse_finder_2d_both.pyexamples/collapse_finder_2d_irv.pyexamples/collapse_finder_2d_tvr.pytests/test_collapse_2d_shared.pytests/test_collapse_finder_2d_both.pytests/test_collapse_finder_2d_irv.pytests/test_collapse_finder_2d_tvr.py
| def plot_favorability_bar(ax, favorability_pct, labels, colors, fg, grid): | ||
| """Plot mean normalized utility as an average favorability percentage.""" | ||
| bars = ax.bar( | ||
| range(len(labels)), | ||
| favorability_pct, | ||
| tick_label=list(labels), | ||
| color=colors, | ||
| ) | ||
| for rect in bars: | ||
| height = rect.get_height() | ||
| if height > 0: | ||
| ax.annotate( | ||
| f'{height:.0f}', | ||
| xy=(rect.get_x() + rect.get_width() / 2, height), | ||
| xytext=(0, 3), | ||
| textcoords='offset points', | ||
| ha='center', | ||
| va='bottom', | ||
| color=fg, | ||
| ) | ||
| ax.set_ylim(0, 100) | ||
| ax.set_ylabel('Mean utility [%]') | ||
| ax.grid(True, alpha=0.25, axis='y', color=grid) | ||
| ax.set_axisbelow(True) | ||
| ax.text( | ||
| 0.5, | ||
| 1.04, | ||
| 'Average favorability', | ||
| transform=ax.transAxes, | ||
| ha='center', | ||
| va='center', | ||
| color=fg, | ||
| ) | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the utility label wording.
The y-axis label reads 'Mean utility [%]'. The panel title reads 'Average favorability'. Both describe the same metric. Use one term consistently so viewers of the animation do not read this as two different measures.
💡 Proposed fix for consistent terminology
ax.text(
0.5,
1.04,
- 'Average favorability',
+ 'Mean utility',
transform=ax.transAxes,
ha='center',
va='center',
color=fg,
)📝 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.
| def plot_favorability_bar(ax, favorability_pct, labels, colors, fg, grid): | |
| """Plot mean normalized utility as an average favorability percentage.""" | |
| bars = ax.bar( | |
| range(len(labels)), | |
| favorability_pct, | |
| tick_label=list(labels), | |
| color=colors, | |
| ) | |
| for rect in bars: | |
| height = rect.get_height() | |
| if height > 0: | |
| ax.annotate( | |
| f'{height:.0f}', | |
| xy=(rect.get_x() + rect.get_width() / 2, height), | |
| xytext=(0, 3), | |
| textcoords='offset points', | |
| ha='center', | |
| va='bottom', | |
| color=fg, | |
| ) | |
| ax.set_ylim(0, 100) | |
| ax.set_ylabel('Mean utility [%]') | |
| ax.grid(True, alpha=0.25, axis='y', color=grid) | |
| ax.set_axisbelow(True) | |
| ax.text( | |
| 0.5, | |
| 1.04, | |
| 'Average favorability', | |
| transform=ax.transAxes, | |
| ha='center', | |
| va='center', | |
| color=fg, | |
| ) | |
| def plot_favorability_bar(ax, favorability_pct, labels, colors, fg, grid): | |
| """Plot mean normalized utility as an average favorability percentage.""" | |
| bars = ax.bar( | |
| range(len(labels)), | |
| favorability_pct, | |
| tick_label=list(labels), | |
| color=colors, | |
| ) | |
| for rect in bars: | |
| height = rect.get_height() | |
| if height > 0: | |
| ax.annotate( | |
| f'{height:.0f}', | |
| xy=(rect.get_x() + rect.get_width() / 2, height), | |
| xytext=(0, 3), | |
| textcoords='offset points', | |
| ha='center', | |
| va='bottom', | |
| color=fg, | |
| ) | |
| ax.set_ylim(0, 100) | |
| ax.set_ylabel('Mean utility [%]') | |
| ax.grid(True, alpha=0.25, axis='y', color=grid) | |
| ax.set_axisbelow(True) | |
| ax.text( | |
| 0.5, | |
| 1.04, | |
| 'Mean utility', | |
| transform=ax.transAxes, | |
| ha='center', | |
| va='center', | |
| color=fg, | |
| ) |
🤖 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 `@examples/collapse_2d_shared.py` around lines 163 - 196, Align the y-axis
terminology in plot_favorability_bar with the panel title by replacing the “Mean
utility” wording in ax.set_ylabel with “Average favorability” while preserving
the percentage unit.
| matplotlib.use('Agg') | ||
| import matplotlib.pyplot as plt | ||
| import numpy as np | ||
| from PIL import Image |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate dependency manifests and check for a Pillow declaration.
fd -H -t f -e toml -e cfg -e txt -e yml -e yaml --max-depth 3 \
| xargs rg -n -i --no-heading 'pillow|^\s*PIL\b|matplotlib|numpy' || true
# Show declared dependency sections for context.
fd -H -t f 'pyproject.toml|setup.cfg|setup.py|requirements.*\.txt' --max-depth 3 --exec sh -c 'echo "== $1 =="; cat -n "$1"' _ {}Repository: endolith/elsim
Length of output: 2620
🏁 Script executed:
# Find the test file mentioned in the review comment
fd -type f 'test_collapse_finder_2d_tvr.py'
# Check if the example file exists
fd -type f 'collapse_finder_2d_tvr.py'
# Look at pytest configuration
rg -n 'pytest|examples' pyproject.tomlRepository: endolith/elsim
Length of output: 787
🏁 Script executed:
# Read the test file if it exists
if [ -f tests/test_collapse_finder_2d_tvr.py ]; then
echo "=== Test file content ==="
cat -n tests/test_collapse_finder_2d_tvr.py
else
echo "Test file not found"
fi
# Check the example file
if [ -f examples/collapse_finder_2d_tvr.py ]; then
echo "=== Example file content (first 25 lines) ==="
head -n 25 examples/collapse_finder_2d_tvr.py
else
echo "Example file not found"
fiRepository: endolith/elsim
Length of output: 5615
🏁 Script executed:
# Check what matplotlib depends on and if Pillow is required
python3 - <<'PY'
# Check metadata of matplotlib to confirm Pillow is a dependency
# Since we can't query installed packages, let's look for documentation in the repo
import subprocess
result = subprocess.run(['rg', '-n', 'pillow|PIL', '--', '.'], capture_output=True, text=True)
print("Pillow references in codebase:")
print(result.stdout if result.stdout else "No matches found")
PYRepository: endolith/elsim
Length of output: 308
Add matplotlib to test extras to enable the test.
The test file (tests/test_collapse_finder_2d_tvr.py) imports matplotlib at line 3, but matplotlib is declared only in the examples extras, not in the test extras. When you run tests in a clean environment with only test dependencies, the test fails on the matplotlib import before reaching the Pillow import. Add matplotlib to the test extras in pyproject.toml to make the test runnable. Pillow will be available as a transitive dependency of matplotlib.
🤖 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 `@examples/collapse_finder_2d_tvr.py` at line 16, Add matplotlib to the test
extras dependency list in pyproject.toml, while preserving the existing examples
extras declaration, so tests/test_collapse_finder_2d_tvr.py can import
matplotlib in a clean test-only environment.
| for step in range(frames_per_transfer): | ||
| start = step * per_frame | ||
| stop = min(start + per_frame, n_voters) | ||
| for voter in order[start:stop]: | ||
| higher = higher_ranked[voter] | ||
| running_borda[higher] -= 1 | ||
| running_borda[loser] -= n_active - len(higher) | ||
| transferred = np.flatnonzero(transferred_voters == voter) | ||
| if len(transferred): | ||
| running_choices[voter] = transferred_to[transferred[0]] |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Precompute the transfer lookup; the current search is quadratic in voters.
np.flatnonzero(transferred_voters == voter) scans the whole transferred_voters array for every voter, inside a loop that visits every voter once per round. The cost is O(n_voters²) per round. The __main__ block uses n_voters = 5000 and n_cands = 9, so this runs about eight rounds of 25 million comparisons, plus 5000 separate NumPy calls per round.
Build the voter-to-target map once per round instead.
⚡ Proposed fix for the quadratic lookup
order = rng.permutation(n_voters)
per_frame = max(1, ceildiv(n_voters, frames_per_transfer))
running_borda = round_.borda_before.copy()
running_choices = choices.copy()
+ transfer_target = np.full(n_voters, -1, dtype=np.int64)
+ transfer_target[transferred_voters] = transferred_to
for step in range(frames_per_transfer):
start = step * per_frame
stop = min(start + per_frame, n_voters)
for voter in order[start:stop]:
higher = higher_ranked[voter]
running_borda[higher] -= 1
running_borda[loser] -= n_active - len(higher)
- transferred = np.flatnonzero(transferred_voters == voter)
- if len(transferred):
- running_choices[voter] = transferred_to[transferred[0]]
+ target = transfer_target[voter]
+ if target >= 0:
+ running_choices[voter] = targetThis also removes the reliance on transferred[0] when a voter appears more than once in transferred_voters.
📝 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.
| for step in range(frames_per_transfer): | |
| start = step * per_frame | |
| stop = min(start + per_frame, n_voters) | |
| for voter in order[start:stop]: | |
| higher = higher_ranked[voter] | |
| running_borda[higher] -= 1 | |
| running_borda[loser] -= n_active - len(higher) | |
| transferred = np.flatnonzero(transferred_voters == voter) | |
| if len(transferred): | |
| running_choices[voter] = transferred_to[transferred[0]] | |
| order = rng.permutation(n_voters) | |
| per_frame = max(1, ceildiv(n_voters, frames_per_transfer)) | |
| running_borda = round_.borda_before.copy() | |
| running_choices = choices.copy() | |
| transfer_target = np.full(n_voters, -1, dtype=np.int64) | |
| transfer_target[transferred_voters] = transferred_to | |
| for step in range(frames_per_transfer): | |
| start = step * per_frame | |
| stop = min(start + per_frame, n_voters) | |
| for voter in order[start:stop]: | |
| higher = higher_ranked[voter] | |
| running_borda[higher] -= 1 | |
| running_borda[loser] -= n_active - len(higher) | |
| target = transfer_target[voter] | |
| if target >= 0: | |
| running_choices[voter] = target |
🤖 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 `@examples/collapse_finder_2d_tvr.py` around lines 294 - 303, In the
round-processing loop around `transferred_voters`, precompute a voter-to-target
lookup once before iterating through voters, then retrieve each voter's target
directly instead of calling `np.flatnonzero` per voter. Preserve the existing
update to `running_choices` only for voters present in the transfer map, and
define a deterministic value for duplicate voter entries without relying on
`transferred[0]`.
There is no such distinction. Both methods stop on first-choice majority. Ned Foley's claim that they differ on this is incorrect. Devin was also wrong to say "Baldwin is not Condorcet-consistent", which led to my "Bruh" comment above. So currently this PR is rejected as-is for that reason. I did not check everything else for correctness. |
|
Tip For best results, initiate chat on the files or code changes.
Both Baldwin and Total Vote Runoff stop when a candidate has a first-choice majority. This behavior does not distinguish the methods. Baldwin is Condorcet-consistent. The prior statement that it was not Condorcet-consistent was incorrect. I am sorry for the incorrect review and the wasted review cost. 🐇 ✏️ Learnings added
You are interacting with an AI system. |
It is not limited to this project. That is in the original definitions of the systems. Are you able to re-review this PR and double check for factual accuracy on issues like this? (I have not used CodeRabbit before.) |
|
I will check election-method claims against authoritative definitions and the implementation. I will distinguish verified facts from uncertain claims. I will not treat the repository implementation as the source for method definitions. I corrected the learning scope. The first-choice-majority stop rule is part of the definitions of both Baldwin and Total Vote Runoff. Baldwin is Condorcet-consistent. 🐇 ✏️ Learnings added
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
examples/collapse_finder_2d_irv.py (2)
311-323: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the shared frame-cleanup and GIF-assembly code into
examples/collapse_2d_shared.py. Both example scripts contain byte-identical output plumbing that differs only in the GIF file name. The root cause is that this common plumbing was not placed in the shared module, so future fixes must be applied twice.
examples/collapse_finder_2d_irv.py#L311-L323: replace the GIF-assembly block with a call to a new shared helper, for examplewrite_gif(output_dir, durations, 'collapse_2d_irv.gif').examples/collapse_finder_2d_irv.py#L170-L173: delete_clear_png_framesand import the shared version fromexamples.collapse_2d_shared.examples/collapse_finder_2d_tvr.py#L200-L203: delete the identical_clear_png_framesand import the shared version.examples/collapse_finder_2d_tvr.py#L350-L362: replace the GIF-assembly block withwrite_gif(output_dir, durations, 'collapse_2d_tvr.gif').🤖 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 `@examples/collapse_finder_2d_irv.py` around lines 311 - 323, Move the shared _clear_png_frames and GIF assembly into examples/collapse_2d_shared.py by adding a write_gif helper that accepts output_dir, durations, and the GIF filename, preserving frame cleanup. In examples/collapse_finder_2d_irv.py lines 311-323 and examples/collapse_finder_2d_tvr.py lines 350-362, replace the duplicated assembly blocks with write_gif calls using their respective filenames. In examples/collapse_finder_2d_irv.py lines 170-173 and examples/collapse_finder_2d_tvr.py lines 200-203, remove the local _clear_png_frames definitions and import the shared implementation.
18-29: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMerge the duplicate import from
examples.collapse_2d_shared.Line 29 imports
count_winsfrom the same module already imported at lines 18-28.♻️ Proposed refactor
candidate_name, + count_wins, create_frame_scaffold, get_palette_colors, prepare_palette_and_labels, sort_candidates_bell_curve, transition_step_ms, ) -from examples.collapse_2d_shared import count_wins🤖 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 `@examples/collapse_finder_2d_irv.py` around lines 18 - 29, Merge the separate count_wins import into the existing examples.collapse_2d_shared import block, keeping all imported symbols and removing the duplicate module import.examples/collapse_2d_shared.py (1)
75-97: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
NamedTuplefor the theme colors.
get_themereturns eight anonymous colors. Every caller unpacks all eight in order, and tests index with[-1]. ANamedTuplekeeps tuple unpacking and index access, and it makes each color self-documenting.♻️ Proposed refactor sketch
+class Theme(NamedTuple): + bg: object + fg: object + grid: object + stroke_fg: object + legend_bg: object + legend_fg: object + voronoi: object + dead_zone: object + def get_theme(dark_background): """Return colors for the dark or light rendering theme.""" if dark_background: - return ( + return Theme( 'black',🤖 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 `@examples/collapse_2d_shared.py` around lines 75 - 97, Refactor get_theme to return a NamedTuple with descriptive field names for all eight theme values, preserving the existing field order so callers can continue tuple-unpacking and tests can continue indexing with [-1]. Update any necessary type or import declarations without changing the selected colors or behavior.
🤖 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 `@examples/collapse_2d_shared.py`:
- Around line 225-229: Update the Voronoi construction in the overlay-rendering
function around Voronoi(points) to catch scipy.spatial QhullError for collinear
or duplicate active points and return immediately. Ensure the early return
occurs before any overlay drawing while preserving the existing rendering path
for valid point sets.
In `@examples/README.md`:
- Around line 328-331: Update the `collapse_finder_2d_both.py` description in
`examples/README.md` to remove the claim that it renders animations side by
side, and instead state that it writes separate GIFs to the `irv/` and `tvr/`
subdirectories while using the same voters and candidates.
---
Nitpick comments:
In `@examples/collapse_2d_shared.py`:
- Around line 75-97: Refactor get_theme to return a NamedTuple with descriptive
field names for all eight theme values, preserving the existing field order so
callers can continue tuple-unpacking and tests can continue indexing with [-1].
Update any necessary type or import declarations without changing the selected
colors or behavior.
In `@examples/collapse_finder_2d_irv.py`:
- Around line 311-323: Move the shared _clear_png_frames and GIF assembly into
examples/collapse_2d_shared.py by adding a write_gif helper that accepts
output_dir, durations, and the GIF filename, preserving frame cleanup. In
examples/collapse_finder_2d_irv.py lines 311-323 and
examples/collapse_finder_2d_tvr.py lines 350-362, replace the duplicated
assembly blocks with write_gif calls using their respective filenames. In
examples/collapse_finder_2d_irv.py lines 170-173 and
examples/collapse_finder_2d_tvr.py lines 200-203, remove the local
_clear_png_frames definitions and import the shared implementation.
- Around line 18-29: Merge the separate count_wins import into the existing
examples.collapse_2d_shared import block, keeping all imported symbols and
removing the duplicate module import.
🪄 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: f4691627-eb66-42f5-8be1-90648b854c89
📒 Files selected for processing (10)
.gitignoreexamples/README.mdexamples/collapse_2d_shared.pyexamples/collapse_finder_2d_both.pyexamples/collapse_finder_2d_irv.pyexamples/collapse_finder_2d_tvr.pytests/test_collapse_2d_shared.pytests/test_collapse_finder_2d_both.pytests/test_collapse_finder_2d_irv.pytests/test_collapse_finder_2d_tvr.py
| voronoi = Voronoi(points) | ||
| center = points.mean(axis=0) | ||
| xlim = ax.get_xlim() | ||
| ylim = ax.get_ylim() | ||
| span = max(np.ptp(xlim), np.ptp(ylim)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the SciPy version pinned by the project and the import path for QhullError.
fd -H -t f 'pyproject.toml|setup.cfg|setup.py|requirements.*\.txt' --max-depth 3 --exec sh -c 'echo "== $1 =="; rg -n "scipy|python" "$1"' _ {}Repository: endolith/elsim
Length of output: 220
🌐 Web query:
scipy.spatial QhullError import path documentation
💡 Result:
To access QhullError in SciPy, you should import it directly from the scipy.spatial namespace [1][2]: from scipy.spatial import QhullError QhullError is the exception raised by SciPy when the underlying Qhull library encounters an error, such as a geometrical degeneracy [1][3]. It is exposed in the scipy.spatial namespace as of SciPy version 1.8 [4]. If you encounter an "ImportError: cannot import name 'QhullError' from 'scipy.spatial'" error, it typically indicates that you are using an older version of SciPy (prior to v1.8) [4]. Upgrading your SciPy installation to a more recent version is the recommended solution [4].
Citations:
- 1: https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.QhullError.html
- 2: https://docs.scipy.org/doc/scipy/reference/spatial.html
- 3: https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.ConvexHull.html
- 4: cannot import name 'QhullError' from 'scipy.spatial' scipy/scipy#19096
🏁 Script executed:
#!/bin/bash
set -eu
echo "== target file outline =="
ast-grep outline examples/collapse_2d_shared.py --lang python
echo "== imports and relevant definitions =="
rg -n -C 8 '^(from scipy\.spatial|import scipy)|def voronoi_plot_2d_axes|def create_frame_scaffold|Voronoi\(|candidates\[active\]' examples/collapse_2d_shared.py
echo "== dependency context =="
rg -n -C 3 'scipy|requires-python' pyproject.toml requirements\* 2>/dev/null || trueRepository: endolith/elsim
Length of output: 4396
🌐 Web query:
SciPy Voronoi QhullError collinear duplicate points behavior and scipy.spatial.QhullError documentation
💡 Result:
The scipy.spatial.QhullError is an exception raised by SciPy when the underlying Qhull library encounters a geometric degeneracy or processing error that it cannot resolve [1][2]. In the context of Voronoi diagrams and other spatial computations, this typically occurs due to issues like collinear points, duplicate points, or high-precision/scale discrepancies that exceed the tolerances of the algorithm [3][4][5]. Behavior and Handling: - Geometrical Degeneracy: When points are collinear or duplicated, numerical precision issues may prevent Qhull from including them as vertices [3]. Such points are often classified as coplanar and may be listed in the coplanar attribute of the resulting object if computed [2][6]. - Qhull Options: Users can influence how Qhull handles these cases by passing custom strings to the qhull_options parameter [7][8]. For example, the "QJ" option instructs Qhull to randomly perturb input data to resolve degeneracies [3]. - Practical Troubleshooting: - Rescaling and Re-centering: If coordinates are very far from the origin relative to the differences between individual points, precision errors often arise. Subtracting the mean or rescaling the input data to a more standard range is a common and effective workaround [4][5]. - Avoiding Crashing: In rare cases where Qhull triggers an internal exit (often resulting in an uncatchable error or specific exit codes), modifying the input slightly or adjusting qhull_options (e.g., removing "Qbb" if it conflicts with specific geometry) may prevent the crash [9][5]. Official Documentation: The scipy.spatial.QhullError class is documented as a RuntimeError specifically for when Qhull encounters unresolved geometrical conditions [1][2]. Documentation for individual classes like Voronoi notes that the library default options are generally "Qbb Qc Qz" (for low dimensions), which helps manage most common precision issues, but users may need to adjust these strings for specific edge cases [7][10].
Citations:
- 1: https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.QhullError.html
- 2: https://github.com/scipy/scipy/blob/main/scipy/spatial/_qhull.pyx
- 3: https://docs.scipy.org/doc/scipy-1.4.1/reference/tutorial/spatial.html
- 4: https://stackoverflow.com/questions/56654686/scipy-griddata-qhullerror-center-point-is-coplanar-with-a-facet-or-a-vertex-is
- 5: BUG: spatial.Voronoi: qhull internal error (qh_order_vertexneighbors): no neighbor of v4 for f39 scipy/scipy#23263
- 6: https://docs.scipy.org/doc/scipy-1.8.0/reference/generated/scipy.spatial.ConvexHull.html
- 7: https://docs.scipy.org/doc/scipy/reference/generated/scipy.spatial.Voronoi.html
- 8: https://docs.scipy.org/doc/scipy-1.15.3/reference/generated/scipy.spatial.Voronoi.html
- 9: BUG: spatial:
voronoihas internal issue with Qhull and infinite edges? scipy/scipy#24231 - 10: https://docs.scipy.org/doc/scipy-1.14.0/reference/generated/scipy.spatial.Voronoi.html
🏁 Script executed:
#!/bin/bash
set -eu
echo "== Voronoi helper =="
sed -n '198,265p' examples/collapse_2d_shared.py
echo "== scaffold active-set flow =="
sed -n '300,380p' examples/collapse_2d_shared.py
echo "== runtime exception probe =="
python3 - <<'PY'
import numpy as np
from scipy.spatial import Voronoi, QhullError
cases = {
"collinear": np.array([[0., 0.], [1., 0.], [2., 0.]]),
"duplicate": np.array([[0., 0.], [1., 1.], [1., 1.]]),
"nondegenerate": np.array([[0., 0.], [1., 0.], [0., 1.]]),
}
print("QhullError:", QhullError.__module__, QhullError.__mro__)
for name, points in cases.items():
try:
Voronoi(points)
except Exception as exc:
print(name, type(exc).__module__ + "." + type(exc).__name__)
else:
print(name, "success")
PYRepository: endolith/elsim
Length of output: 5290
Guard the Voronoi construction against degenerate point sets.
When active candidates are collinear or duplicated, Voronoi(points) raises QhullError and aborts the frame render. Catch QhullError and return before drawing the overlay.
🛡️ Proposed fix
-from scipy.spatial import Voronoi
+from scipy.spatial import QhullError, Voronoi
- voronoi = Voronoi(points)
+ try:
+ voronoi = Voronoi(points)
+ except QhullError:
+ return📝 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.
| voronoi = Voronoi(points) | |
| center = points.mean(axis=0) | |
| xlim = ax.get_xlim() | |
| ylim = ax.get_ylim() | |
| span = max(np.ptp(xlim), np.ptp(ylim)) | |
| try: | |
| voronoi = Voronoi(points) | |
| except QhullError: | |
| return | |
| center = points.mean(axis=0) | |
| xlim = ax.get_xlim() | |
| ylim = ax.get_ylim() | |
| span = max(np.ptp(xlim), np.ptp(ylim)) |
🤖 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 `@examples/collapse_2d_shared.py` around lines 225 - 229, Update the Voronoi
construction in the overlay-rendering function around Voronoi(points) to catch
scipy.spatial QhullError for collinear or duplicate active points and return
immediately. Ensure the early return occurs before any overlay drawing while
preserving the existing rendering path for valid point sets.
| - **Both** (`collapse_finder_2d_both.py`): finds a single election satisfying | ||
| both verified criteria above: IRV squeezes out the Condorcet winner while | ||
| Baldwin correctly elects it. It renders both animations side by side from | ||
| the same voters and candidates. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the "side by side" description.
examples/collapse_finder_2d_both.py writes two separate GIFs into the irv/ and tvr/ subdirectories. It does not compose one frame with both animations. Describe the actual output so readers do not expect a combined layout.
📝 Proposed wording fix
- **Both** (`collapse_finder_2d_both.py`): finds a single election satisfying
both verified criteria above: IRV squeezes out the Condorcet winner while
- Baldwin correctly elects it. It renders both animations side by side from
- the same voters and candidates.
+ Baldwin correctly elects it. It renders both animations from the same voters
+ and candidates into separate `irv/` and `tvr/` subdirectories.📝 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.
| - **Both** (`collapse_finder_2d_both.py`): finds a single election satisfying | |
| both verified criteria above: IRV squeezes out the Condorcet winner while | |
| Baldwin correctly elects it. It renders both animations side by side from | |
| the same voters and candidates. | |
| - **Both** (`collapse_finder_2d_both.py`): finds a single election satisfying | |
| both verified criteria above: IRV squeezes out the Condorcet winner while | |
| Baldwin correctly elects it. It renders both animations from the same voters | |
| and candidates into separate `irv/` and `tvr/` subdirectories. |
🤖 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 `@examples/README.md` around lines 328 - 331, Update the
`collapse_finder_2d_both.py` description in `examples/README.md` to remove the
claim that it renders animations side by side, and instead state that it writes
separate GIFs to the `irv/` and `tvr/` subdirectories while using the same
voters and candidates.
bf15c21 to
68a3d3f
Compare
3976b9e to
cbb45b5
Compare
68a3d3f to
f3518f3
Compare
Provide reusable candidate ordering, pairwise-win, integer-division, candidate-label, and transition-timing helpers for the two-dimensional collapse examples. Implement the two-point Voronoi perpendicular bisector from a midpoint and segment-normal direction so vertical and horizontal layouts are safe. Cover the pure geometry helpers with focused tests. Co-authored-by: Endolith <endolith@gmail.com>
Centralize palette loading, candidate-label preparation, grayscale filtering, and dark/light rendering colors for the collapse examples. Include the Baldwin rank-panel dead-zone colors in the theme contract from the beginning so all consumers share the same configuration. Co-authored-by: Endolith <endolith@gmail.com>
Build the common scatter, favorability, and head-to-head panels used by the IRV and Baldwin animations. Keep the method-specific middle panel open, use visible −σ/0/σ ticks, and label normalized utility as average favorability rather than approval. Thread the complete theme contract, including the Baldwin dead-zone color, through the scaffold. Co-authored-by: Endolith <endolith@gmail.com>
Add the IRV animation on top of the frozen round-trace API and shared frame scaffold. Reject eager zero-vote exclusions, validate the strict center-outward order, and require the first eliminated center candidate to be the actual Condorcet winner from the pairwise matrix. Reconstruct transfer states deterministically and cover the validator and GIF rendering path with focused tests. Co-authored-by: Endolith <endolith@gmail.com>
Add the full Baldwin elimination animation using the frozen round-trace API and exact voter-wise Borda transitions. Preserve the regression-tested score replay and one-based average-rank display, while requiring the geometric center to be the actual Condorcet winner from the pairwise matrix. Document Baldwin and Total Vote Runoff Condorcet consistency, and cover score replay, finder validation, and GIF rendering. Co-authored-by: Endolith <endolith@gmail.com>
Add a thin driver that finds one shared spatial election where the verified Condorcet winner is eliminated first by IRV but elected by Baldwin. Reuse the sibling validators and animation entry points, preserve the shared voters and rankings, and test the combined traces and pairwise winner. Co-authored-by: Endolith <endolith@gmail.com>
Ignore the repository-local results directory used by the 2D collapse animations so generated PNG frames, GIFs, and saved positions never enter source control. Co-authored-by: Endolith <endolith@gmail.com>
Document the verified Condorcet contrast in the 2D examples: IRV can eliminate the pairwise winner through center squeeze, while Baldwin and Total Vote Runoff satisfy the Condorcet criterion and elect it. Describe the pairwise-matrix verification performed by the scripts and the shared generated-output location. Co-authored-by: Endolith <endolith@gmail.com>
cbb45b5 to
2e7e797
Compare
|
@endolith Both of your corrections are right — thank you. Fixed in base PR #71 (
Corrected. Baldwin/Nanson/TVR do satisfy the Condorcet criterion — a Condorcet winner always has above-average Borda, so it's never the minimum and is never eliminated. The finders now verify this from elsim's own pairwise matrix (
Corrected. The first-choice-majority stop is now unconditional in the shared count, so Separately, I rebased #71 onto current |
1 participant
Summary
Clean rebuild of the 2D center-squeeze collapse example scripts from the
collapse_2dbranch. The scripts now consume the frozen round-trace dataclasses added in the base PR (#71) instead of reimplementing method transitions inside the rendering code, and several correctness bugs from the original scripts are fixed.Stacked on #71 — review that first; this PR targets its branch as the base, so the diff here is only the example/animation layer.
Correctness fixes vs the original
collapse_2dscriptslen(promoted_per_voter[v])and added+1to each below-loser candidate, so the loser's bar never decayed to 0 and intermediate bars disagreed with a real retally. Now the transition is driven directly from the trace using the exact one-based Borda accounting:avg_rank = n_active - borda/n_voters(gave rank 0 for a first-place candidate and a bar overshooting then_cands-1y-limit). Now one-based:avg_rank = (n_active + 1) - borda/n_voters, so best = rank 1, worst active = rankn_active, and bars stay within the axis.condorcet_from_matrix(ranked_election_to_matrix(...))) — it keeps an election only when a Condorcet winner exists, is the geometric-center candidate, and is the one Baldwin elects. The IRV finder verifies the candidate it squeezes out first is that same verified Condorcet winner (the center-squeeze failure). Earlier revisions of this PR incorrectly claimed Baldwin was not Condorcet-consistent; that was wrong and is corrected here.IRVResult.initially_eliminated(not a clean center-squeeze) and validatesround.eliminatedagainst the nearest still-active candidate, ending on the two farthest (stop_at=2).2*(x2-x1), blowing up when two candidates share an x (candidate 0 sits at the origin). Replaced with a general perpendicular-bisector construction.Structure / quality
examples/collapse_2d_shared.pyholds a single four-panel frame scaffold (create_frame_scaffold) plus geometry/palette/theme helpers, so the IRV and TVR renderers no longer duplicate layout code; each only supplies its middle panel (votes% for IRV, avg-rank/Borda for TVR).__main__and passes them explicitly.__main__guards; no hardcoded paths outside the repo; generated GIFs/PNGs are written underexamples/results/and git-ignored (not committed — they exceed the pre-commit large-file cap anyway).Scripts
collapse_finder_2d_irv.py— IRV center-outward collapse that eliminates the verified Condorcet winner first.collapse_finder_2d_tvr.py— Baldwin / Total Vote Runoff elimination collapse to the verified Condorcet winner.collapse_finder_2d_both.py— finds one election satisfying both criteria (IRV squeezes out the Condorcet winner; Baldwin elects it) and renders both animations from the same voters/candidates.Tests
New tests (each with a docstring stating what it verifies and why):
tests/test_collapse_2d_shared.py— bell-curve candidate ordering; two-same-x Voronoi no longer raises;count_winson a known pairwise matrix.tests/test_collapse_finder_2d_irv.py— validator accepts a center-first trace, rejects a non-center-first one, and rejects traces with eager zero-vote exclusions; Condorcet verification of the squeezed-out candidate; tiny end-to-end GIF smoke test.tests/test_collapse_finder_2d_tvr.py— regression replay of the per-voter Borda accounting equalsround.borda_afterfor every round; one-based average-rank formula bounds; Condorcet-center validator accept/reject; tiny GIF smoke test.tests/test_collapse_finder_2d_both.py— the combined driver finds a shared election and both traces are non-None.Full suite:
256 passed. Full pre-commit / Ruff gate clean. Both TVR and IRV animations smoke-run end-to-end under the Agg backend and produce a GIF.Related work
Stacked on #71 (methods layer). Merge #71 first; this PR's base is its branch.
Link to Devin session: https://app.devin.ai/sessions/d2d2ac0338354d7ab08c4468ee3e1680
Requested by: @endolith