Fix general bug in GreedySelector that would pick the same point if there are degeneracies

Author: ceriottm

src/skmatter/_selection.py

@@ -427,10 +427,8 @@ def _get_best_new_selection(self, scorer, X, y):
         scores = scorer(X, y)
 
         # Get the score argmax, but only for idxs not already selected
-        _tmp_scores = {
-            i: score for i, score in enumerate(scores) if i not in self.selected_idx_
-        }
-        max_score_idx = max(_tmp_scores, key=_tmp_scores.get)
+        scores[self.selected_idx_[: self.n_selected_]] = -np.inf
+        max_score_idx = np.argmax(scores)
         if self.score_threshold is not None:
             if self.first_score_ is None:
                 self.first_score_ = scores[max_score_idx]

tests/test_feature_pcov_cur.py

@@ -33,7 +33,6 @@ def test_non_it(self):
         self.idx = [2, 8, 3, 6, 7, 9, 1, 0, 5]
         selector = PCovCUR(n_to_select=9, recompute_every=0)
         selector.fit(self.X, self.y)
-
         self.assertTrue(np.allclose(selector.selected_idx_, self.idx))
 
 

tests/test_feature_simple_cur.py

@@ -50,8 +50,8 @@ def test_unique_selected_idx_zero_score(self):
         n_samples = 10
         n_features = 15
         X = np.random.rand(n_samples, n_features)
-        X[:, 3] = np.random.rand(10) * 1e-13
-        X[:, 4] = np.random.rand(10) * 1e-13
+        X[:, 1] = X[:, 0]
+        X[:, 2] = X[:, 0]
         selector_problem = CUR(n_to_select=len(X.T)).fit(X)
         assert len(selector_problem.selected_idx_) == len(
             set(selector_problem.selected_idx_)

tests/test_feature_simple_fps.py

@@ -93,8 +93,8 @@ def test_unique_selected_idx_zero_score(self):
         n_samples = 10
         n_features = 15
         X = np.random.rand(n_samples, n_features)
-        X[:, 3] = np.random.rand(10) * 1e-13
-        X[:, 4] = np.random.rand(10) * 1e-13
+        X[:, 1] = X[:, 0]
+        X[:, 2] = X[:, 0]
         selector_problem = FPS(n_to_select=len(X.T)).fit(X)
         assert len(selector_problem.selected_idx_) == len(
             set(selector_problem.selected_idx_)

tests/test_sample_simple_cur.py

@@ -61,9 +61,9 @@ def test_unique_selected_idx_zero_score(self):
         n_samples = 10
         n_features = 15
         X = np.random.rand(n_samples, n_features)
-        X[4, :] = np.random.rand(15) * 1e-13
-        X[5, :] = np.random.rand(15) * 1e-13
-        X[6, :] = np.random.rand(15) * 1e-13
+        X[1] = X[0]
+        X[2] = X[0]
+        X[3] = X[0]
         selector_problem = CUR(n_to_select=len(X)).fit(X)
         assert len(selector_problem.selected_idx_) == len(
             set(selector_problem.selected_idx_)

tests/test_sample_simple_fps.py

@@ -108,9 +108,9 @@ def test_unique_selected_idx_zero_score(self):
         n_samples = 10
         n_features = 15
         X = np.random.rand(n_samples, n_features)
-        X[4, :] = np.random.rand(15) * 1e-13
-        X[5, :] = np.random.rand(15) * 1e-13
-        X[6, :] = np.random.rand(15) * 1e-13
+        X[1] = X[0]
+        X[2] = X[0]
+        X[3] = X[0]
         selector_problem = FPS(n_to_select=len(X)).fit(X)
         assert len(selector_problem.selected_idx_) == len(
             set(selector_problem.selected_idx_)

tests/test_voronoi_fps.py

@@ -165,6 +165,22 @@ def test_score(self):
             )
         )
 
+    def test_unique_selected_idx_zero_score(self):
+        """
+        Tests that the selected idxs are unique, which may not be the
+        case when the score is numerically zero
+        """
+        np.random.seed(0)
+        n_samples = 10
+        n_features = 15
+        X = np.random.rand(n_samples, n_features)
+        X[1] = X[0]
+        X[2] = X[0]
+        selector_problem = VoronoiFPS(n_to_select=n_samples, initialize=3).fit(X)
+        assert len(selector_problem.selected_idx_) == len(
+            set(selector_problem.selected_idx_)
+        )
+
 
 if __name__ == "__main__":
     unittest.main(verbosity=2)