Batch comparison: Add extra batches once per round - #2346
Conversation
There was a problem hiding this comment.
Pull request overview
Refactors the experimental “extra batch” selection for batch comparison audits so extra batches are chosen once per round (across all audited contests) instead of independently per contest, reducing unnecessary additional batch auditing in multi-contest audits.
Changes:
- Removes
round_numfromcompute_sample_batches_for_contest()and returns only the contest’s core sample. - Introduces
compute_extra_batches_for_round()to compute extra batches using the combined round sample across contests. - Updates
compute_sample_batches()to append round-wide extra batches after per-contest sampling.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server/api/shared.py | Extracts extra-batch logic into a round-wide function and adjusts sampling flow to avoid per-contest extra selection. |
| .basedpyright/baseline.json | Updates the basedpyright baseline (likely due to new/changed typing surfaces from the refactor). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| for jurisdiction in jurisdictions: | ||
| representative_contest_id = jurisdiction_id_to_contest_id[jurisdiction.id] | ||
| batch_key_to_num_ballots = { | ||
| (jurisdiction.name, batch.name): batch.num_ballots |
There was a problem hiding this comment.
@jonahkagan I think the comment is correct, though it seems to predate this PR. sampled_batch_results() gets all batches which have batch tally sheets for this contest, and then when excluding non-rla batches, it only filters out extra batches that were tagged for this contest. But if contest B pulls in an extra batch, there will be a tally sheet with contest A for that batch, which ends up included in the sampled_rows.
I think this fixes it, by removing all extra batches except any that also live in the rla set (if a batch gets chosen as extra in first round, but part of sample in second round)
| def compute_extra_batches_for_round( | ||
| election: Election, | ||
| round_num: int, | ||
| contest_sample_sizes: list[tuple[Contest, SampleSize]], | ||
| sampled_batches: list[BatchDraw], | ||
| ) -> list[BatchDraw]: |
fdc63fb to
e2d6e1b
Compare
|
@jonahkagan I put together this simpler fix for this ticket, as my other PR (mentioned in PR description) ended up being having more surface area than I expected, and I don't feel as confident about getting it done by Thursday. Can you take a look if this approach seems reasonable for now, and then I'll finalize the logic/testing? |
jonahkagan
left a comment
jonahkagan
left a comment
There was a problem hiding this comment.
Approach seems reasonable to me. The comment from copilot seems worth looking into
… when flag is set
| batch_key: result | ||
| for batch_key, result in results.items() | ||
| if batch_key not in extra_batch_keys | ||
| if batch_key not in (extra_batch_keys - rla_sampled_batch_keys) |
There was a problem hiding this comment.
Why do we need to subtract out rla_sampled_batch_keys here? I didn't think it was possible for an RLA-sampled batch to also be used as an extra batch
There was a problem hiding this comment.
That was considering a multi-round audit that has a batch selected as an extra in round 1, but then part of the sample in round 2, as I think that sequencing can happen
There was a problem hiding this comment.
Oh, interesting edge case. Cool
|
Did a manual test with the GA election from May and saw intended results. In my local snapshot, I cleared the batch tallies and undid the round start. Upon redrawing, every jurisdiction saw their number of extra batches stay the same or go down. Looked into some jurisdictions with the largest changes (along with some Claude investigation) and they were correctly sampled. Feeling confident enough to merge. |
#2324
Georgia noticed in their recent audit that more extra batches were selected than needed, and this was because extra batches where being added once per contest instead of once per round. Batches audited for one contest are already audited for the other contest, so we don't need extra batches added for each.
This PR makes a low-lift effort to refactor the extra batch selection to make it once per round instead of per contest.
This subsequent PR suggests data model changes to move extra batches out of the experimental state and into official support, but given we are close to the audit day (Thursday), I think it's safer to just go with the lower lift change for now.
Testing:
Will do a manual test with the GA extra batch flag turned on for multi-contest.
Will add a multi-contest extra batch test.