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
Allow single-winner threshold to be based on first-round votes only #646
Allow single-winner threshold to be based on first-round votes only #646
Conversation
1fc3842
to
7af25a3
Compare
7af25a3
to
e824b17
Compare
Is this one ready for review? |
@tarheel yep |
…single-winner-threshold
d7a6e55
to
e79080a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition to (or instead of) the test case you created, could you include one in which the winner fails to reach the threshold? I just want to confirm that that case works, since (I believe) it violates an assumption that we previously held about how thresholds work.
@@ -158,7 +158,18 @@ Set<String> tabulate() throws TabulationAbortedException { | |||
// votes in the first round. | |||
// In a single-seat contest or in the special multi-seat bottoms-up threshold mode, it's based | |||
// on the number of active votes in the current round. | |||
if (currentRound == 1 || config.getNumberOfWinners() <= 1) { | |||
boolean doRecomputeThreshold; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nit: I think shouldRecomputeThreshold
would be more clear
Modified test so threshold isn't reached, but winner is still selected based on being the sole remaining candidate. This may not be the desired behavior: the rounds continue until just one candidate remains, rather than picking whoever has the most votes when two candidates remain. I'm not sure if this is the desired behavior? Should nobody be elected? Should somebody be elected when two candidates remain? |
311c1d6
to
8300593
Compare
8300593
to
8c02dba
Compare
@chughes297 can probably confirm, but I believe the desired behavior would be to stop when there are two candidates remaining and elect the candidate with more votes. On a related note: it just occurred to me that I'm not 100% sure we handle it properly if you end up with two candidates remaining and they're tied. Ordinarily we go to the tie-breaker logic, but I'm not sure that accounts for this scenario... |
The tiebreaker logic occurs to figure out which of the two candidates to eliminate, but not which of the two to elect. The tabulation still runs down to just one candidate. I've added another commit with a test for this so you can see how it works. Would love to hear what Chris thinks. Chris, any word on whether it's okay to run down to one candidate when using this rule? |
Eliminating the second-place candidate might be necessary to push the first-place candidate over the threshold, so this seems to me to be desirable. But is there a description somewhere of how this would be an impactful change? There must be some other consequence planned if they haven’t reached the threshold, e.g. nobody elected, rerun the election, etc., because once a candidate reaches a majority of the vote they will always have a majority of the vote. |
Ha, you would think. But apparently it's just something that's now required in Minnesota even though it has no impact on the outcome. |
Ah, but if you eliminate the 2nd candidate, the winner is now guaranteed to always have unanimous support! 🤣 |
Thinking more about this, I can see that this requirement of surpassing the original threshold is going to result in strategic voting in the form of candidates encouraging their supporters to “rank me first and don’t rank anyone else, because the more exhausted ballots the less likely it is that another candidate might win”. A bad, bad idea, Minnesota. |
I was randomly looking at issues and saw this. Note that even if you eliminate the 2nd-place out of two candidates left, it's still possible that the winner won't reach the threshold established in the first round (for example, if none of the ballots for the losing candidates rank the other candidate further down). |
f12da08
to
666db28
Compare
@cjerdonek I agree, but I think this has some bizarre consequences. I've added a commit which feels very dirty to me, but is probably the correct way of doing things...
Here's a visualization, with a table below. Is this what we want? This feels better than leaving the last-vote-getter uneliminated if they didn't reach the threshold, since that looks too similar to the last-vote-getter being elected. @chughes297 can you look at these visualizations and let me know if you think that's how the code should react when nobody reaches the threshold? |
This, from upthread, is the desired behavior. It's functionally only a change in how the results information is reported - all the Minnesota RCV jurisdictions require the results to report winners based on the threshold set by valid first-round votes. It's essentially IRV reported using STV threshold-setting rules. Which means that even if no one crosses threshold the person with the most votes at the end wins. The final round is always a head-to-head matchup. Minneapolis Mayor 2021 Results example: https://vote.minneapolismn.gov/media/-www-content-assets/documents/2021-Mayor-Round-Results-Summary.pdf. Sorry for not including an example originally! Would have saved a lot of back and forth. |
Oh sorry just noticed the tiebreaker question - I would treat a top-two-candidates tie as a tie between candidates to be elected. Does that answer that question? |
Sorry, I'm not exactly sure what you mean. I think the desired behavior should be to run the tie-breaker and then stop and declare the winner of the tie-breaker to be the election winner. (In any earlier round, we would run the tie-breaker and then eliminate the loser, but that doesn't make sense in this context because it would lead to an additional round in which only the winner remains.) And I haven't confirmed, but I don't believe that's what will happen with the code as it currently stands. |
Yes, the desired behavior is to run the tie-breaker and then stop and declare the winner of the tie-breaker to be the election winner. I agree it will look weird in the results as people discussed elsewhere, but that's something that's better left to a text-based explanation included with the results, IMO. |
This reverts commit 666db28.
I reverted the last commit to return to the original functionality: when there are two remaining candidates, the one with the most votes wins. We can check tiebreaker consistency as part of #666. I believe all comments here are now resolved? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll rely on @tarheel to give the final approval, since he was much closer to this than I was. Just a couple questions from my end.
ab7b78f
to
05235e3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the only potential issue here is that when we end up with two candidates remaining and neither one meets the threshold, we're not actually supposed to generate a final round in which only the winning candidate remains; instead, the tabulation should behave the same way it does when the winner meets the threshold, i.e. they get elected and the tabulation stops without eliminating the runner-up. This has no impact on who wins, of course, but IIRC it's a nitpicky detail that's been flagged in the past by someone or other. And ditto if we need a tie-breaker to determine the winner at this point.
You can confirm with @chughes297 that my understanding here is accurate...
src/main/resources/network/brightspots/rcv/hints_winning_rules.txt
Outdated
Show resolved
Hide resolved
@tarheel to get us all on the same page -- Current BehaviorYour Suggestion@chughes297 etc, can you chime in? Which of these should RCTab produce? |
05235e3
to
eb5a352
Compare
Except in the second image I would want Vail to have the green highlight in the last round to indicate that he won. |
agree! |
6c41305
to
4b88733
Compare
4b88733
to
3154b29
Compare
// Edge case: if nobody meets the threshold, but we're on the penultimate round when | ||
// isFirstRoundDeterminesThresholdEnabled is true, select the max vote getters as | ||
// the winners. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your comment could also note that if isFirstRoundDeterminesThresholdEnabled
isn't enabled, it should be impossible for a single-winner election to end up here.
// defer the others to subsequent rounds. If this is a single-winner election where its | ||
// possible for no candidate to reach the threshold (i.e. "first round determines threshold" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super nits: s/where its/in which it's/
and s/ / /
// Edge case: if we've identified multiple winners in this round, but we're only supposed to | ||
// elect one winner per round, pick the top vote-getter and defer the others to subsequent | ||
// rounds. | ||
if (config.isMultiSeatAllowOnlyOneWinnerPerRoundEnabled() && selectedWinners.size() > 1) { | ||
// elect one winner per round, pick the top vote-getter. If this is a multi-winner election, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for clarity, make this a bullet list:
Edge cases:
- (Case 1)
- (Case 2)
boolean useTiebreakerIfNeeded = config.isMultiSeatAllowOnlyOneWinnerPerRoundEnabled() | ||
|| config.isFirstRoundDeterminesThresholdEnabled(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the case in which it's single-winner and isFirstRoundDeterminesThresholdEnabled
is false but we end up with two candidates who both have exactly half of the votes?
If that's not already handled, we should add a test for it, and I would recommend tweaking this to something like:
boolean tiebreakNeeded = selectedWinners.size() > 1
&& (config.getNumberOfWinners() == 1 || config.isMultiSeatAllowOnlyOneWinnerPerRoundEnabled());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would lead to one candidate being eliminated, so it wouldn't be handled here -- since nobody has reached the threshold, the tiebreaker runs at the elimination stage.
The test for this is in tiebreak_seed_test
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so I think that test is wrong! It eliminates the tiebreak loser and runs a final round with only the winner continuing. I think we want to make that behavior consistent with what you've done here. @chughes297 can confirm.
To be specific, here's what that test is currently expecting:
{
"config" : {
"contest" : "Tiebreak test",
"date" : "2017-12-03",
"jurisdiction" : "Funkytown, USA",
"office" : "Sergeant-at-Arms",
"threshold" : "2"
},
"results" : [ {
"round" : 1,
"tally" : {
"George Gervin" : "3",
"Mookie Blaylock" : "3",
"Yinka Dare" : "3"
},
"tallyResults" : [ {
"eliminated" : "Yinka Dare",
"transfers" : {
"exhausted" : "3"
}
} ]
}, {
"round" : 2,
"tally" : {
"George Gervin" : "3",
"Mookie Blaylock" : "3"
},
"tallyResults" : [ {
"eliminated" : "George Gervin",
"transfers" : {
"exhausted" : "3"
}
} ]
}, {
"round" : 3,
"tally" : {
"Mookie Blaylock" : "3"
},
"tallyResults" : [ {
"elected" : "Mookie Blaylock",
"transfers" : { }
} ]
} ]
}
And I think what we actually want is for round 2 to be the final round, and in that round no one is eliminated but we elect Blaylock via the tiebreaker.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And I think what we actually want is for round 2 to be the final round, and in that round no one is eliminated but we elect Blaylock via the tiebreaker.
Yes!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To keep this issue well-scoped, and since that's a bug that exists in develop
today, I've created a separate issue to address it: #694
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll plan to give this a quick review on the UI stuff once @tarheel is satisfied with everything.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved for UI changes.
Single-Winner races (enabled):
Multi-Winner races (disabled):
Included a simple test to verify.