Skip to content

Not possible to create a Ranking object with one-indexed permutation #2

@scottfleming

Description

@scottfleming

If I try to run the following code from the main directory:

from code.core import ws_ranking
from code.core.ws_ranking import WeakSupRanking
from code.core import ranking_utils
from code.core.ranking_utils import RankingUtils, Ranking

r_utils = RankingUtils(3)
r = Ranking(permutation = [1, 2, 3], r_utils=tmp_utils)
r_utils.ranking_to_pair_signs(r)

I get an error:

---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[4], line 2
      1 r_utils = RankingUtils(3)
----> 2 r = Ranking(permutation = [1, 2, 3], r_utils=tmp_utils)
      3 r_utils.ranking_to_pair_signs(r)

File ~/Documents/GitHub/WS-Structured-Prediction/code/core/ranking_utils.py:471, in Ranking.__init__(self, permutation, r_utils, z)
    468 self.mask = np.ones(len(self.r_utils.unique_pairs))
    470 if z is None:
--> 471     self.z = self.r_utils.ranking_to_pair_signs(self)
    472 else:
    473     self.z = z

File ~/Documents/GitHub/WS-Structured-Prediction/code/core/ranking_utils.py:78, in RankingUtils.ranking_to_pair_signs(self, r)
     76             z[idx] = 1 * r.mask[idx]
     77         else:
---> 78             idx = self.pair_index_map[self.pair_key(b, a)]
     79             z[idx] = -1 * r.mask[idx]
     80 return z

KeyError: '3,1'

Two issues here:
(1) It feels awkward that RankingUtils requires as part of its initialization the dimension of a Ranking object, but then the initialization of a Ranking object requires a RankingUtils object -- this feels like a circular dependency.
(2) The error seems to arise because in lines 28 through 39 of ranking_utils.py (see below), self.pair_index_map is set to have zero-indexed pair keys rather than arbitrary rank values that might come from eg a Ranking object's permutation (see circular dependency issue above). So despite the fact that a and b in ranking_to_pair_signs is set to take on the arbitrary values from within the Ranking object, the self.pair_index_map object never had access to those values when it was being initialized. In practice, the way I see it, this means that Ranking objects can only ever take on zero-indexed values. This feels like a design flaw, but more importantly...
(3) None of these assumptions are documented in the class descriptors or docstrings for Ranking and RankingUtils, nor are there any unit tests that I could use to make sure nothing breaks were I to try and submit a fix.

self.items = list(range(d))
self.unique_pairs = []
for i in range(d):
for j in range(i + 1, d):
self.unique_pairs.append((i, j))
self.pair_index_map = {}
k = 0
for i in range(d):
for j in range(i + 1, d):
self.pair_index_map[self.pair_key(self.items[i], self.items[j])] = k
k += 1

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions