Fix Hare Quota

[NCGThompson]

Background: In most variants of RCV, a candidates tally must exceed (i.e. greater than) the threshold to automatically win. Hare quota is an exception, where the tally must meet or exceed (greater than or equal to) the threshold. Internally, the tabulator sets the variable winningThreshold with the function setWinningThreshold(). The program determines a candidate to be a winner if the tally is greater than or equal to winningThreshold. Note that the tally is always a discreet value in this software, even if it is a decimal.

Since the tally is measured to a certain precision, it can be thought of as on a discreet number line. If we aren't using the hare quota, then we want to set winningThreshold to a value where the tally >= winningThreshold if and only if the tally is greater than the actual threshold. We achieve this by finding the first point on the number line that is to the right of the exact threshold, and setting winningThreshold to that. However, before this commit, setWinningThreshold() worked the same way for hare quotient. For hare quotas, we should choose the first value on the number line that is on or to the right of the exact threshold to find our winningThreshold.

(Note that an exception is if nonIntegerWinningThreshold is false. In this case the tally may exceed the exact threshold being less than winningThreshold; but, since this is an explicit user configuration, it is still correct.)

augend is the distance between each point on the number line. decimals also indicates the precision of the number line, set in the config file. RoundingMode.UP means round up and RoundingMode.DOWN means round down. a.divide(b, ...) means a divided by b, with the specified rounding. In this case a/b is the exact threshold.

For non-hare quotas, this expression finds winningThreshold on line 387:

winningThreshold = currentRoundTotalVotes.divide(divisor, decimals, RoundingMode.DOWN).add(augend);

and hare on 383:

winningThreshold = currentRoundTotalVotes.divide(divisor, decimals, RoundingMode.UP);

This pull request changes the code for handling nonIntegerWinningThreshold. Previously there were two separate paths in an if else statement depending on the value of nonIntegerWinningThreshold. This commit consolidates the two paths. If the variable is true, then decimals is set to zero, and it is as if the number line only had integers. Otherwise decimals is set to decimalPlacesForVoteArithmetic. This is done in lines 370-373.

[NCGThompson]

This addresses #557

[tarheel]

Thanks for the contribution! I think your logic looks right... but this part of the code is always tricky to reason about. Have you been able to get the test suite working? We already have tests for non-Hare configs (test_set_multi_winner_fractional_threshold and test_set_multi_winner_whole_threshold), but we don't have one for Hare yet (which helps to explain why this bug exists in the first place).

Also, were you able to apply Checkstyle to your changes? I don't see anything that's clearly inconsistent, but sometimes it's hard to predict what it will do...

[HEdingfield]

Echoing @tarheel's comment, I'd appreciate it if you also included a test that fails with the status quo and passes once this fix is applied.

No need to worry too much about Checkstyle though, as I run this before we do any merging to our main branch.

[NCGThompson]

Have you been able to get the test suite working?

No. I have found the TabulatorTests class is source, but I don't know how to run the tests.

[tarheel]

@HEdingfield @moldover do you know how to run the tests from the command line? We should figure that out and add it to the README.

[HEdingfield]

$ gradle test
or:
$ ./gradlew test
should do it.

[NCGThompson]

I successfully ran the tests, but most of them failed. That was because the outputted threshold had trailing zeros that the reference did not have. Commit 6214e5f fixed this.

After that only one test failed: test Hare quota (i.e. 2013 Minneapolis park hare quota). That output a different result in the way it was supposed to, by using first integer greater than or equal to rather than greater than. All I need to do is change the expected output for the unit test, and I will have

a test that fails with the status quo and passes once this fix is applied.

[NCGThompson]

Updated the unit test test in c4a04c7!

Try $ ./gradlew test --tests TabulatorTests.testHareQuota

[NCGThompson]

As of a3f4884, the pull request passes $ ./gradlew check with the exception of unrelated errors already existing before the pull request. That should be fixed in a different pull request.

Now all of @tarheel and @HEdingfield's recommendations are accounted for.

[tarheel]

[HEdingfield]

Nice work, thanks!

[NCGThompson]

Don't wait for me to pull it, I don't have write access.