Skip to content

Conversation

gnossen
Copy link
Contributor

@gnossen gnossen commented Jul 25, 2025

No description provided.

- 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'
Copy link
Member

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.
Copy link
Member

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:
Copy link
Member

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
Copy link
Member

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"

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change


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.
Copy link
Member

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.
Copy link
Member

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>

Comment on lines +255 to +267
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)
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Remove nesting

Suggested change
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.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// 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.",
Copy link
Member

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

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

Successfully merging this pull request may close these issues.

2 participants