Repository: scikit-activeml/scikit-activeml (language: python)

Issue:
More restrictive check_cost_matrix?
- Should each cost matrix contain at least one value > 0?
- Should check_cost_matrix allow negative costs?


Discussion / hints:
We could add additional parameters:

- `contains_non_zero` with default `False`,
- `only_non_negative` with default `False'.

What do you think @dakot @christoph14 ?


Fix patch:
diff --git a/skactiveml/base.py b/skactiveml/base.py
index ffaf7fb1..ed38673d 100644
--- a/skactiveml/base.py
+++ b/skactiveml/base.py
@@ -1,11 +1,12 @@
-import numpy as np
 import warnings
-
 from abc import ABC, abstractmethod
+
+import numpy as np
 from sklearn.base import BaseEstimator, ClassifierMixin, clone
+from sklearn.metrics import accuracy_score
 from sklearn.utils import check_array, check_consistent_length, column_or_1d
 from sklearn.utils.multiclass import type_of_target
-from sklearn.metrics import accuracy_score
+
 from skactiveml.utils import MISSING_LABEL, check_classifier_params, \
     check_random_state, rand_argmin, ExtLabelEncoder, check_cost_matrix, \
     is_labeled, check_scalar
@@ -435,8 +436,8 @@ def _validate_data(self, X, y, sample_weight):
         # Update cost matrix.
         self.cost_matrix_ = 1 - np.eye(len(self.classes_)) \
             if self.cost_matrix is None else self.cost_matrix
-        self.cost_matrix_ = np.array(check_cost_matrix(self.cost_matrix_,
-                                                       len(self.classes_)))
+        self.cost_matrix_ = check_cost_matrix(self.cost_matrix_,
+                                              len(self.classes_))
         if self.classes is not None:
             class_indices = np.argsort(self.classes)
             self.cost_matrix_ = self.cost_matrix_[class_indices]
diff --git a/skactiveml/utils/_validation.py b/skactiveml/utils/_validation.py
index a813ee3f..0f4e5e7a 100644
--- a/skactiveml/utils/_validation.py
+++ b/skactiveml/utils/_validation.py
@@ -1,8 +1,8 @@
-import numpy as np
-import sklearn
-
 from collections.abc import Iterable
 
+import numpy as np
+import sklearn
+import warnings
 from sklearn.utils.validation import check_array, column_or_1d, \
     assert_all_finite, check_consistent_length
 
@@ -144,7 +144,8 @@ def check_classes(classes):
                 types))
 
 
-def check_cost_matrix(cost_matrix, n_classes):
+def check_cost_matrix(cost_matrix, n_classes, only_non_negative=False,
+                      contains_non_zero=False, diagonal_is_zero=False):
     """Check whether cost matrix has shape `(n_classes, n_classes)`.
 
     Parameters
@@ -153,6 +154,15 @@ def check_cost_matrix(cost_matrix, n_classes):
         Cost matrix.
     n_classes : int
         Number of classes.
+    only_non_negative : bool, optional (default=True)
+        This parameter determines whether the matrix must contain only non
+        negative cost entries.
+    contains_non_zero : bool, optional (default=True)
+        This parameter determines whether the matrix must contain at least on
+        non-zero cost entry.
+    diagonal_is_zero : bool, optional (default=True)
+        This parameter determines whether the diagonal cost entries must be
+        zero.
 
     Returns
     -------
@@ -166,6 +176,35 @@ def check_cost_matrix(cost_matrix, n_classes):
         raise ValueError(
             "'cost_matrix' must have shape ({}, {}). "
             "Got {}.".format(n_classes, n_classes, cost_matrix_new.shape))
+    if np.sum(cost_matrix_new < 0) > 0:
+        if only_non_negative:
+            raise ValueError(
+                "'cost_matrix' must contain only non-negative cost entries."
+            )
+        else:
+            warnings.warn(
+                "'cost_matrix' contains negative cost entries."
+            )
+    if n_classes != 1 and np.sum(cost_matrix_new != 0) == 0:
+        if contains_non_zero:
+            raise ValueError(
+                    "'cost_matrix' must contain at least one non-zero cost "
+                    "entry."
+                )
+        else:
+            warnings.warn(
+                "'cost_matrix' contains contains no non-zero cost entry."
+            )
+    if np.sum(np.diag(cost_matrix_new) != 0) > 0:
+        if diagonal_is_zero:
+            raise ValueError(
+                "'cost_matrix' must contain only cost entries being zero on "
+                "its diagonal."
+            )
+        else:
+            warnings.warn(
+                "'cost_matrix' contains non-zero cost entries on its diagonal."
+            )
     return cost_matrix_new
 
 


Test patch:
diff --git a/skactiveml/pool/tests/test_alce.py b/skactiveml/pool/tests/test_alce.py
index 437e1687..ab155706 100644
--- a/skactiveml/pool/tests/test_alce.py
+++ b/skactiveml/pool/tests/test_alce.py
@@ -1,6 +1,6 @@
-import numpy as np
 import unittest
 
+import numpy as np
 from sklearn.svm import SVR
 
 from skactiveml.classifier import PWC
@@ -14,7 +14,7 @@ def setUp(self):
         self.X = np.zeros((6, 2))
         self.y = [0, 1, 1, 0, 2, 1]
         self.classes = [0, 1, 2]
-        self.cost_matrix = np.array([[1, 2, 3], [4, 5, 6], [7, 8, 9]])
+        self.cost_matrix = np.array([[0, 2, 3], [4, 0, 6], [7, 8, 0]])
         self.regressor = SVR()
         self.pwc = PWC()
 
diff --git a/skactiveml/utils/tests/test_validation.py b/skactiveml/utils/tests/test_validation.py
index d2408ca8..0dea3ad4 100644
--- a/skactiveml/utils/tests/test_validation.py
+++ b/skactiveml/utils/tests/test_validation.py
@@ -1,5 +1,6 @@
-import numpy as np
 import unittest
+import numpy as np
+import warnings
 
 from skactiveml.utils import check_cost_matrix, check_classes, \
     check_missing_label, check_scalar, check_X_y
@@ -25,11 +26,27 @@ def test_check_cost_matrix(self):
         self.assertRaises(ValueError, check_cost_matrix,
                           cost_matrix=[['2', '5'], ['a', '5']], n_classes=2)
         self.assertRaises(ValueError, check_cost_matrix,
-                          cost_matrix=[[2, 1], [2, 2]], n_classes=3)
+                          cost_matrix=[[0, 1], [2, 0]], n_classes=3)
         self.assertRaises(ValueError, check_cost_matrix,
-                          cost_matrix=[[2, 1], [2, 2]], n_classes=-1)
+                          cost_matrix=[[0, 1], [2, 0]], n_classes=-1)
         self.assertRaises(TypeError, check_cost_matrix,
-                          cost_matrix=[[2, 1], [2, 2]], n_classes=2.5)
+                          cost_matrix=[[0, 1], [2, 0]], n_classes=2.5)
+        self.assertRaises(ValueError, check_cost_matrix,
+                          cost_matrix=[[2, 1], [2, 2]], n_classes=2,
+                          diagonal_is_zero=True)
+        self.assertRaises(ValueError, check_cost_matrix,
+                          cost_matrix=[[0, 1], [-1, 0]], n_classes=2,
+                          only_non_negative=True)
+        self.assertRaises(ValueError, check_cost_matrix,
+                          cost_matrix=[[0, 0], [0, 0]], n_classes=2,
+                          contains_non_zero=True)
+        with warnings.catch_warnings(record=True) as w:
+            warnings.simplefilter("always")
+            check_cost_matrix(cost_matrix=[[1, 1], [2, 0]], n_classes=2)
+            check_cost_matrix(cost_matrix=[[0, 1], [-1, 0]], n_classes=2)
+            check_cost_matrix(cost_matrix=[[0, 0], [0, 0]], n_classes=2)
+            assert len(w) == 3
+
 
     def test_check_classes(self):
         self.assertRaises(TypeError, check_classes, classes=[None, 1, 2])