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

Issue:
RandomState not really random
check if randomstate None is really random
tests fail after check_random_state is fixed
After a modification in check_random_state, the following tests fail:

- [x] test_rand_argmax
- [x] test_rand_argmin
- [x] test_al_cycle
- [x] test_pwc
- [x] test_cmm

Branch: 90-fix-check_random_state


Discussion / hints:

Fix patch:
diff --git a/skactiveml/utils/_selection.py b/skactiveml/utils/_selection.py
index d145830a..32c58f1d 100644
--- a/skactiveml/utils/_selection.py
+++ b/skactiveml/utils/_selection.py
@@ -2,7 +2,7 @@
 
 import numpy as np
 
-from sklearn.utils import check_random_state
+from skactiveml.utils import check_random_state
 
 
 def rand_argmin(a, random_state=None, **argmin_kwargs):
diff --git a/skactiveml/utils/_validation.py b/skactiveml/utils/_validation.py
index 0f4e5e7a..eb769a10 100644
--- a/skactiveml/utils/_validation.py
+++ b/skactiveml/utils/_validation.py
@@ -1,3 +1,4 @@
+import copy
 from collections.abc import Iterable
 
 import numpy as np
@@ -395,13 +396,35 @@ def check_X_y(X, y, X_cand=None, sample_weight=None, sample_weight_cand=None,
         return X, y, X_cand, sample_weight, sample_weight_cand
 
 
-def check_random_state(random_state, seed_multiplier=1):
-    # Check given random state
-    random_state = sklearn.utils.check_random_state(random_state)
+def check_random_state(random_state, seed_multiplier=None):
+    """Checks validity of random state and creates a new random state for
+    a given seed multiplier.
 
-    # Check multiplier
-    check_scalar(seed_multiplier, 'seed_multiplier', int, min_val=1)
+    Parameters
+    ----------
+    random_state : None | int | instance of RandomState
+        If random_state is None, return the RandomState singleton used by
+        np.random.
+        If seed is an int, return a new RandomState.
+        If seed is already a RandomState instance, return it.
+        Otherwise raise ValueError.
+    seed_multiplier : None | int, optional (default=None)
+        If the random_state and seed_multiplier are not None, the seed
+        multiplier is multiplied with the seed of the random_state.
+
+    Returns
+    -------
+    random_state: instance of RandomState
+        The validated random state.
+    """
+    if random_state is None or seed_multiplier is None:
+        return sklearn.utils.check_random_state(random_state)
+
+    check_scalar(seed_multiplier, name='seed_multiplier', target_type=int,
+                 min_val=1)
+    random_state = copy.deepcopy(random_state)
+    random_state = sklearn.utils.check_random_state(random_state)
 
-    seed = random_state.get_state()[1][0]
+    seed = (random_state.randint(2**31)*seed_multiplier) % (2**31)
+    return np.random.RandomState(seed)
 
-    return np.random.RandomState((seed * seed_multiplier) % (2 ** 32))


Test patch:
diff --git a/skactiveml/pool/tests/test_pool.py b/skactiveml/pool/tests/test_pool.py
index c3ccbaf1..c009801a 100644
--- a/skactiveml/pool/tests/test_pool.py
+++ b/skactiveml/pool/tests/test_pool.py
@@ -39,7 +39,7 @@ def test_al_cycle(self):
             with self.subTest(msg="Random State", qs_name=qs_name):
                 y = np.full(self.y_true.shape, self.MISSING_LABEL)
                 qs = call_func(
-                    self.query_strategies[qs_name], only_mandatory=True,
+                    self.query_strategies[qs_name], only_mandatory=False,
                     clf=clf, classes=np.unique(self.y_true),
                     random_state=np.random.RandomState(0))
 
diff --git a/skactiveml/utils/tests/test_validation.py b/skactiveml/utils/tests/test_validation.py
index 0dea3ad4..bb58156a 100644
--- a/skactiveml/utils/tests/test_validation.py
+++ b/skactiveml/utils/tests/test_validation.py
@@ -73,13 +73,24 @@ def test_check_X_y(self):
 
     def test_check_random_state(self):
         seed = 12
-        multiplier = 3
         self.assertRaises(ValueError, check_random_state, 'string')
         self.assertRaises(TypeError, check_random_state, seed, 'string')
 
-        random_state = check_random_state(seed, seed_multiplier=multiplier)
-        new_seed = random_state.get_state()[1][0]
-        self.assertEqual(new_seed, seed * multiplier)
+        random_state = np.random.RandomState(seed)
+        ra = check_random_state(random_state, 3)
+        rb = check_random_state(random_state, 3)
+        self.assertTrue(ra.rand() == rb.rand())
+
+        ra = check_random_state(42, 3)
+        rb = check_random_state(42, 3)
+        self.assertTrue(ra.rand() == rb.rand())
+
+        ra = check_random_state(None)
+        rb = check_random_state(None)
+        self.assertTrue(ra.rand() != rb.rand())
+        ra = check_random_state(np.random.RandomState(None))
+        rb = check_random_state(np.random.RandomState(None))
+        self.assertTrue(ra.rand() != rb.rand())
 
 
 if __name__ == '__main__':