Skip to content

Faster mentions#367

Open
EdouardHeitzmann wants to merge 7 commits into
mggg:3.4.1from
EdouardHeitzmann:feature/better_mentions
Open

Faster mentions#367
EdouardHeitzmann wants to merge 7 commits into
mggg:3.4.1from
EdouardHeitzmann:feature/better_mentions

Conversation

@EdouardHeitzmann
Copy link
Copy Markdown
Collaborator

I got tired of waiting >5 seconds every time I need to compute mentions for large profiles.

The legacy mentions function in votekit/utils uses the Profile's .ballots property to compute ballots; when this property is not already set, it is computed and cached when mentions is called. This is quite slow when we are passing a large profile that was loaded via a cvr loader that relies on a pandas df rather than a list of ballots to create the profile. This PR introduces a new pandas-only way to compute mentions in such a case, which is around 14-20x faster on my machine for the Portland profiles.

When the ballot list is already materialized, the legacy mentions are faster; in such a case the fast_mentions function I'm adding defaults back to said legacy function.

I also add a few extra tests for both the old and the new mentions functions to verify intended behavior on profiles with repeated and tied rankings (although the former is probably not the intended use case).

Some things worth considering as part of this PR:

  • I kept the old mentions calculator as the one named mentions here, and I named my addition fast_mentions; we might just want to replace mentions completely instead.
  • This might be a good opportunity to allow compatibility with Fraction outputs, if we care (cf. Issue Add back in Fraction support #358). I am happy to do this if desired (this would involve me chasing down other places mentions are used in votekit and tinkering with the typing).

@EdouardHeitzmann
Copy link
Copy Markdown
Collaborator Author

Ok well it seems like the 3.11 tests are broken -- I'm not sure if this is my doing (and if so, how I did that). Lmk

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.

1 participant