-
Notifications
You must be signed in to change notification settings - Fork 35
Elections tooling #34
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
base: main
Are you sure you want to change the base?
Conversation
- run: echo "The name of the branch is ${{ github.ref }} and the repository is ${{ github.repository }}." | ||
- uses: actions/setup-go@v5 | ||
with: | ||
go-version: '1.24.x' |
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.
Assume this will not get updated for years. Is there a go-stable-latest
or something that can be used instead?
return scoreSumMatrixInternal(sumMatrix, placements, winnerCount, map[int]bool{}, []int{}) | ||
} | ||
|
||
// TODO: Test this function. |
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.
👀
|
||
// TODO: Test this function. | ||
|
||
// Returns: |
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.
The linter won't like your comment.
It's a good idea to run the various checkers on your code, since it can sometimes identify actual issues.
@@ -0,0 +1,308 @@ | |||
package score |
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.
Needs a package comment.
Also copyright?
Somewhere there should be a description of the scoring rules this is applying. Either here or linking into the governance or something.
"fmt" | ||
"maps" | ||
"math" | ||
|
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.
|
||
if tie { | ||
if len(sumMatrix)-len(removedCandidates)-len(least) >= winnerCount { | ||
// If the choice of which one of these to eliminate doesn't affect the overall result, we can eliminate them all. |
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'm not familiar with the details of the algorithm you're using here.
But if it doesn't matter which one you eliminate, why not do this first, and skip calling findEliminatee
?
And at that point, why both with the heads-up stuff and just eliminate least
immediately?
} | ||
return scoreSumMatrixInternal(sumMatrix, placements, winnerCount, removedCandidates, losers) | ||
} else { | ||
// TODO: This section is probably too deeply nested now. |
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.
It's easy to un-nest your things.
if !tie {
// We have a single definitive candidate to eliminate.
...
}
if len(sumMatrix)-len(removedCandidates)-len(least) >= winnerCount {
// If the choice of which one of these to eliminate doesn't affect the overall result, we can eliminate them all.
...
}
<now this logic>
for c := 0; c < len(sumMatrix); c++ { | ||
if _, ok := removedCandidates[c]; !ok { | ||
isTie := false | ||
for _, l := range least { | ||
if c == l { | ||
isTie = true | ||
} | ||
} | ||
if !isTie { | ||
winners = append(winners, c) | ||
} | ||
} | ||
} |
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.
Remove nesting
for c := 0; c < len(sumMatrix); c++ { | |
if _, ok := removedCandidates[c]; !ok { | |
isTie := false | |
for _, l := range least { | |
if c == l { | |
isTie = true | |
} | |
} | |
if !isTie { | |
winners = append(winners, c) | |
} | |
} | |
} | |
for c := 0; c < len(sumMatrix); c++ { | |
if _, ok := removedCandidates[c]; ok { | |
// Already removed; ignore. | |
continue | |
} | |
isTie := false | |
for _, l := range least { | |
if c == l { | |
isTie = true | |
} | |
} | |
if !isTie { | |
winners = append(winners, c) | |
} | |
} |
return f | ||
} | ||
|
||
// Returnst he smallest number of bits required to represent the supplied int. |
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.
// Returnst he smallest number of bits required to represent the supplied int. | |
// Returns the smallest number of bits required to represent the supplied int. |
|
||
var rootCommand = &cobra.Command{ | ||
Use: "tally [csv-file]", | ||
Short: "Tallies results from a gRPC Steering Committee Election using the Condorcet IRV method.", |
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.
Alright, now that I know what you're doing, I still have no idea what you're doing.
Condorcet is typically used for single winner scenarios. However, there's multiple ways of applying it when multiple winners are needed:
https://en.wikipedia.org/wiki/Multiwinner_voting#Condorcet_committees
You say you are using IRV. When I click on "Condorcet IRV" in wikipedia, it takes me to: https://en.wikipedia.org/wiki/Tideman_alternative_method
But that's about single-winner situations.
So what multi-winner algorithm are you implementing? https://www.jstor.org/stable/43662603
No description provided.