Skip to content
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

Fail gracefully when there is no precinct information #650

Merged
merged 9 commits into from
Jun 9, 2023
7 changes: 6 additions & 1 deletion src/main/java/network/brightspots/rcv/ResultsWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,12 @@ void generatePrecinctSummaryFiles(
String precinctFileString = getPrecinctFileString(precinct, filenames);
String outputPath =
getOutputFilePathFromInstance(String.format("%s_precinct_summary", precinctFileString));
int numBallots = numBallotsByPrecinct.get(precinct);
Integer numBallotsObj = numBallotsByPrecinct.get(precinct);
if (numBallotsObj == null) {
Logger.warning("No ballots found in precinct \"%s\"", precinct, numBallotsByPrecinct);
numBallotsObj = 0;
}
int numBallots = numBallotsObj;
generateSummarySpreadsheet(roundTallies, numBallots, precinct, outputPath);
generateSummaryJson(roundTallies, precinctTallyTransfers.get(precinct), precinct, outputPath);
}
Expand Down
34 changes: 24 additions & 10 deletions src/main/java/network/brightspots/rcv/Tabulator.java
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ class Tabulator {
// tracks required winning threshold
private BigDecimal winningThreshold;

Tabulator(List<CastVoteRecord> castVoteRecords, ContestConfig config) {
Tabulator(List<CastVoteRecord> castVoteRecords, ContestConfig config)
throws TabulationAbortedException {
this.castVoteRecords = castVoteRecords;
this.candidateNames = config.getCandidateNames();
this.config = config;
Expand All @@ -91,6 +92,11 @@ class Tabulator {
}

if (config.isTabulateByPrecinctEnabled()) {
if (precinctIds.size() == 0) {
Logger.severe("\"Tabulate by Precinct\" enabled, but CVRs don't list precincts.");
throw new TabulationAbortedException(false);
}

initPrecinctRoundTallies();
}
}
Expand Down Expand Up @@ -841,7 +847,8 @@ private OvervoteDecision getOvervoteDecision(CandidatesAtRanking candidates) {
// logs the results to audit log
// update tallyTransfers counts
private void recordSelectionForCastVoteRecord(
CastVoteRecord cvr, int currentRound, String selectedCandidate, String outcomeDescription) {
CastVoteRecord cvr, int currentRound,
String selectedCandidate, String outcomeDescription) throws TabulationAbortedException {
// update transfer counts (unless there's no value to transfer, which can happen if someone
// wins with a tally that exactly matches the winning threshold)
if (cvr.getFractionalTransferValue().signum() == 1) {
Expand All @@ -851,13 +858,19 @@ private void recordSelectionForCastVoteRecord(
selectedCandidate,
cvr.getFractionalTransferValue());
if (config.isTabulateByPrecinctEnabled()) {
precinctTallyTransfers
.get(cvr.getPrecinct())
.addTransfer(
currentRound,
cvr.getCurrentRecipientOfVote(),
selectedCandidate,
cvr.getFractionalTransferValue());
String precinctId = cvr.getPrecinct();
HEdingfield marked this conversation as resolved.
Show resolved Hide resolved
TallyTransfers precinctTallyTransfer = precinctTallyTransfers.get(precinctId);
if (precinctTallyTransfer == null) {
Logger.severe(
"Precinct \"%s\" is not among the %d known precincts.",
precinctId, precinctIds.size());
throw new TabulationAbortedException(false);
}
precinctTallyTransfer.addTransfer(
currentRound,
cvr.getCurrentRecipientOfVote(),
selectedCandidate,
cvr.getFractionalTransferValue());
}
}

Expand All @@ -879,7 +892,8 @@ private void recordSelectionForCastVoteRecord(
// - exhaust cvrs if they should be exhausted for various reasons
// - assign cvrs to continuing candidates if they have been transferred or in the initial count
// returns a map of candidate ID to vote tallies for this round
private Map<String, BigDecimal> computeTalliesForRound(int currentRound) {
private Map<String, BigDecimal> computeTalliesForRound(int currentRound)
throws TabulationAbortedException {
Map<String, BigDecimal> roundTally = getNewTally();
Map<String, Map<String, BigDecimal>> roundTallyByPrecinct = new HashMap<>();
if (config.isTabulateByPrecinctEnabled()) {
Expand Down
7 changes: 7 additions & 0 deletions src/test/java/network/brightspots/rcv/TabulatorTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -598,4 +598,11 @@ void genericCsvTest() {
void noOneMeetsMinimumTest() {
runTabulationTest("no_one_meets_minimum", TabulationAbortedException.class.toString());
}

@Test
@DisplayName("gracefully fail when tabulate-by-precinct option set without any precincts in CVR")
Copy link
Contributor

Choose a reason for hiding this comment

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

@artoonie said:

I copied and pasted the dominion_no_precinct_data test. I wonder if we should begin allowing non-standalone tests: allow a relative path from one test to another, because this 86,000 line copy-pasta feels terrible. The new test would do well to just point to the other test's data (or to create a shared_test_data folder?)

Agreed, and I like this idea. The only potential issue is that it makes the tests non-hermetic (i.e. changing the input file for one could cause others to unintentionally fail). I don't think this is a good reason not to do this though.

Could you please create a follow-up issue for this (linking to this comment), and maybe we can sneak it into v1.4.0 if it seems relatively low effort?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep! #692

I think we can avoid the first issue by only sharing CVRs in a common directory, so it's clear that those CVRs are shared.

void tabulateByPrecinctWithoutPrecincts() {
runTabulationTest("tabulate_by_precinct_without_precincts",
TabulationAbortedException.class.toString());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
{
"tabulatorVersion" : "TEST",
"outputSettings" : {
"contestName" : "Wyoming Dominion test",
"outputDirectory" : "output",
"contestDate" : "2020-07-19",
"contestJurisdiction" : "Wyoming",
"contestOffice" : "Democratic Primary",
"tabulateByPrecinct" : true,
"generateCdfJson" : false
},
"cvrFileSources" : [ {
"filePath" : "tabulate_by_precinct_without_precincts_input_data",
"contestId" : "1",
"idColumnIndex" : "",
"precinctColumnIndex" : "",
"overvoteDelimiter" : "",
"provider" : "dominion",
"overvoteLabel" : "",
"undervoteLabel" : "",
"undeclaredWriteInLabel" : "12",
"treatBlankAsUndeclaredWriteIn" : false
} ],
"candidates" : [ {
"name" : "Joseph R. Biden",
"excluded" : false,
"aliases" : [ ],
"code" : "1"
}, {
"name" : "Tom Steyer",
"excluded" : false,
"aliases" : [ ],
"code" : "3"
}, {
"name" : "Bernie Sanders",
"excluded" : false,
"aliases" : [ ],
"code" : "4"
}, {
"name" : "Amy Klobuchar",
"excluded" : false,
"aliases" : [ ],
"code" : "5"
}, {
"name" : "Tulsi Gabbard",
"excluded" : false,
"aliases" : [ ],
"code" : "7"
}, {
"name" : "Elizabeth Warren",
"excluded" : false,
"aliases" : [ ],
"code" : "8"
}, {
"name" : "Michael R. Bloomberg",
"excluded" : false,
"aliases" : [ ],
"code" : "9"
}, {
"name" : "Pete Buttigieg",
"excluded" : false,
"aliases" : [ ],
"code" : "11"
} ],
"rules" : {
"tiebreakMode" : "useCandidateOrder",
"overvoteRule" : "exhaustImmediately",
"winnerElectionMode" : "singleWinnerMajority",
"randomSeed" : "",
"numberOfWinners" : "1",
"multiSeatBottomsUpPercentageThreshold" : "",
"decimalPlacesForVoteArithmetic" : "4",
"minimumVoteThreshold" : "0",
"maxSkippedRanksAllowed" : "1",
"maxRankingsAllowed" : "8",
"nonIntegerWinningThreshold" : false,
"hareQuota" : false,
"batchElimination" : true,
"continueUntilTwoCandidatesRemain" : false,
"stopTabulationEarlyAfterRound" : "",
"exhaustOnDuplicateCandidate" : false,
"rulesDescription" : "Wyoming Rules",
"treatBlankAsUndeclaredWriteIn" : false
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"Version": "5.10.11.24",
"List": [
{
"BallotTypeId": 1,
"ContestId": 1
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"Version": "5.10.11.24",
"List": [
{
"Description": "Ballot 1 - Type 1",
"Id": 1,
"ExternalId": ""
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
{
"Version": "5.10.11.24",
"List": [
{
"Description": "Tulsi Gabbard",
"Id": 7,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Pete Buttigieg",
"Id": 11,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Joseph R. Biden",
"Id": 1,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Michael R. Bloomberg",
"Id": 9,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Elizabeth Warren",
"Id": 8,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Amy Klobuchar",
"Id": 5,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Bernie Sanders",
"Id": 4,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "Tom Steyer",
"Id": 3,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
},
{
"Description": "(undeclared)",
"Id": 12,
"ExternalId": "",
"ContestId": 1,
"Type": "Regular"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"IncludeTabulatorFilter": false,
"TabulatorFilterValue": 0,
"IncludeResultContainerFilter": false,
"ResultContainerFilterValue": 0,
"IncludePrecinctPortionFilter": false,
"PrecinctPortionFilterValue": 0,
"IncludeBallotTypeFilter": false,
"BallotTypeFilterValue": 0,
"IncludeContestFilter": false,
"ContestFilterValue": 0,
"SplitFilesPerBatch": false,
"IncludeOnlyPublishedResultContainers": true,
"UseTabulatorFormat": false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"Version": "5.10.11.24",
"List": [
{
"Description": "PRESIDENT OF THE UNITED STATES",
"Id": 1,
"ExternalId": "",
"DistrictId": 2,
"VoteFor": 1,
"NumOfRanks": 5
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"Version": "5.10.11.24",
"List": [
{
"Description": "Election Day",
"Id": 1,
"ExternalId": ""
},
{
"Description": "Vote by Mail",
"Id": 2,
"ExternalId": ""
}
]
}
Loading
Loading