-
Notifications
You must be signed in to change notification settings - Fork 19
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
Granular progress bar #834
Conversation
7b42165
to
358889e
Compare
358889e
to
b56b94e
Compare
b56b94e
to
35b1fe2
Compare
35b1fe2
to
00a2c34
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.
Brilliant job figuring this out!
@@ -1876,7 +1877,7 @@ protected Task<Boolean> createTask() { | |||
@Override | |||
protected Boolean call() { | |||
TabulatorSession session = new TabulatorSession(configPath); | |||
return session.convertToCdf(); | |||
return session.convertToCdf(this::updateProgress); |
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.
Where is this actually used? I don't see any place in the GUI that shows a progress bar for converting to CDF.
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 not used, happy to remove though I don't see any downsides leaving it in either
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.
No problem, fine to leave as-is. Merge away!
from the original issue
As we've discussed, I think both of these take so little time since it is really just dumping in-memory counts onto disc. But just to confirm: is the time it takes to do these included under the progress bar as it's written in this PR? Even if it's not a part of the X/Y calculations, can we confirm that work happens while a progress bar is visible. |
Confirmed: the "tabulate by" calculations happen on a round-by-round basis. It does take slightly longer at the end to write out the tabulate-by files, but that's hardly noticeable. Here's an example with 4400 ballots: With "Tabulate by Precinct"Screen.Recording.2024-06-23.at.12.23.09.PM.movWithout "Tabulate by Precinct"Screen.Recording.2024-06-23.at.12.24.16.PM.mov |
Closes #825
Builds a granular progress bar in the simplest way I could think of:
Tabulation is measured in number of eliminated candidates. This makes Round 1 take longer than other rounds, since more happens in the first round before eliminations begin occurring, but it's still a pretty close approximation.
The benefit of this approach is that we can initialize the full state before reading any CVRs -- we know the maximum number of eliminations.
Of course, we can't know if we can stop early in the progress bar. For example, if a candidate gets over 50% of votes in the first round, there will be zero eliminations. This seems fine to me -- the progress bar will always be a worst-case, and finishing early is okay.
The progress bar also handles the odd case of Multi-Pass IRV, where the same files are read multiple times and the same candidate is eliminated multiple times. However, every test case I have of that is quite small, so while it seems to work, I haven't dug deeply to guarantee it works as intended. Maybe @nurse-the-code could help create a very large multi-pass IRV test case and make sure this works as intended?
Demo:
progressbar-demo.mov