Bug(s) in weak_nanson, possibly all Nanson/Baldwin methods

[mjbaldwin]

Hi Wes and Eric, Michael here.

I've run into a bug in weak_nanson:

File .../pref_voting/pref_voting/iterative_methods.py:1613, in weak_nanson(profile, curr_cands)
   1611 else: 
   1612     num_cands = len(candidates)
-> 1613     updated_rankings = _find_updated_profile(rankings, cands_to_ignore, num_cands)
   1615 while len(winners) == 0: 
   1617     borda_scores = {c: _borda_score(updated_rankings, rcounts, num_cands - cands_to_ignore.shape[0], c) 
   1618                     for c in candidates if not isin(cands_to_ignore, c)}

ValueError: negative dimensions not allowed

Looking through the code, the problem seems to be that on line 1613 (and 1633), rankings has curr_cands candidates, cands_to_ignore is the combination of candidates excluded by curr_cands as well as candidates excluded in previous Nanson iterations, but num_cands only goes up to curr_cands -- so cands_to_ignore can actually be a list that has more elements num_cands, producing the negative number error. (In my case, I was using a profile with 10 candidates, while curr_cands had 6 candidates.)

In the process, I also discovered that the Borda scores (line 1617) seem to be similarly incorrect, calculating the number of candidates as num_cands - cands_to_ignore.shape[0], I think where num_cands ought to be all_num_cands in that case.

I went ahead and forked the project to submit a pull request just for weak_nanson, but then saw that similar but not identical logic is repeated across the _with_explanation() versions, with strict_nanson, and with baldwin. For example, while weak_nanson() initially defines at the start:

cands_to_ignore = np.empty(0) if curr_cands is None else np.array([c for c in profile.candidates if c not in curr_cands])

On the other hand, strict_nanson() defines at the start:

cands_to_ignore = np.empty(0)

And then that seems to introduce its own bug on line 1459, because it's not accounting for curr_cands at all.

So instead of submitting the fix for just weak_nanson or trying to check and fix them all, I'm realizing you may want to standarize on a single logic pattern across all these functions, and I'm not sure which approach you'll prefer for tracking candidates excluded via curr_cands vs. candidates excluded via iteration, and ensuring that candidate removal and Borda score calculations are done with the right numbers of candidates.

(Just for context, I ran into the bug because I'm calculating a ranked list of winners for a profile by calculating the (first-place) winner, applying a tiebreaker as needed, removing them from curr_cands to calculate the (second-place) winner, and so forth.)

[wesholliday]

Thanks, Michael! Could you send an example of the code you ran to produce this error?

[mjbaldwin]

Absolutely, Wes! Here's the exact profile that generated it for me:

import numpy as np
from pref_voting.generate_spatial_profiles import generate_spatial_profile
from pref_voting.utility_functions import linear_utility
from pref_voting.iterative_methods import weak_nanson

np.random.seed((730026589, 1597284812))
sp = generate_spatial_profile(num_cands=10, num_voters=1000, num_dims=2)
up = sp.to_utility_profile(utility_function=linear_utility)
rp = up.to_ranking_profile() 
lp = rp.to_linear_profile()
weak_nanson(lp, curr_cands=[0, 4, 6, 7, 8, 9])

With result:

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
[/tmp/ipython-input-1206970547.py](https://localhost:8080/#) in <cell line: 0>()
      9 rp = up.to_ranking_profile()
     10 lp = rp.to_linear_profile()
---> 11 weak_nanson(lp, curr_cands=[0, 4, 6, 7, 8, 9])

1 frames
[/usr/local/lib/python3.12/dist-packages/pref_voting/iterative_methods.py](https://localhost:8080/#) in weak_nanson(profile, curr_cands)
   1611     else:
   1612         num_cands = len(candidates)
-> 1613         updated_rankings = _find_updated_profile(rankings, cands_to_ignore, num_cands)
   1614 
   1615     while len(winners) == 0:

ValueError: negative dimensions not allowed

[epacuit]

Thanks! I see that borda, strict_nanson, and baldwin do not produce an error on this profile (though we should check others since a similar issue may arise at least for Baldwin and strict_nanson). But, we'll look into and track down the issue.