diff --git a/CHANGELOG.md b/CHANGELOG.md index 09aff22..a2069d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Updated the `randint_untyped` implementation, which now supports `array_like` (broadcastable) `low`/`high` bounds [gh-176](https://github.com/IntelPython/mkl_random/pull/176) ### Fixed +* Fixed an out-of-range integer `brng` indexing `brng_list` past its end, which returned uninitialized memory as random values; it now warns and falls back to `MT19937` [gh-177](https://github.com/IntelPython/mkl_random/pull/177) +* Fixed `brng=0` being treated as unset, which left the state unseeded [gh-177](https://github.com/IntelPython/mkl_random/pull/177) +* Fixed `MKLRandomState(seed, brng=None)` reading past `brng_list` and returning uninitialized memory; it now uses `MT19937` [gh-177](https://github.com/IntelPython/mkl_random/pull/177) * Fixed `uniform` to return a Python `float` for scalar bounds with `size=None` instead of a 0-d array [gh-167](https://github.com/IntelPython/mkl_random/pull/167) * Fixed `randint` returning `high` for `int64`, `uint64` and the default `int` dtype when the range is at or above `INT_MAX` [gh-172](https://github.com/IntelPython/mkl_random/pull/172) * Fixed the integer fills silently under-filling requests larger than two `MKL_INT_MAX` chunks [gh-172](https://github.com/IntelPython/mkl_random/pull/172) diff --git a/mkl_random/mklrand.pyx b/mkl_random/mklrand.pyx index e74fbf5..959b7bf 100644 --- a/mkl_random/mklrand.pyx +++ b/mkl_random/mklrand.pyx @@ -64,6 +64,8 @@ cdef extern from "numpy_multiiter_workaround.h": cdef extern from "randomkit.h": + int BRNG_KINDS + ctypedef struct irk_state: pass @@ -1713,7 +1715,12 @@ cdef irk_brng_t _parse_brng_token_(brng): else: brng_token = tmp elif isinstance(brng, int): - brng_token = operator.index(brng) + # Out of range would index brng_list past its end when seeding. + tmp = operator.index(brng) + if 0 <= tmp < BRNG_KINDS: + brng_token = tmp + else: + brng_token = _default_fallback_brng_token_(brng) else: brng_token = _default_fallback_brng_token_(brng) @@ -1767,6 +1774,9 @@ cdef class _MKLRandomState: self.lock = Lock() self.shuffle_lock = RLock() + # No stream exists yet to take the generator from. + if brng is None: + brng = "MT19937" self._seed_impl(seed, brng) def __dealloc__(self): @@ -1781,8 +1791,10 @@ cdef class _MKLRandomState: cdef unsigned int stream_id cdef cnp.ndarray obj "arrayObject_obj" cdef bint use_array = False + # Not truthiness: 0 is falsy but is MT19937. + cdef bint brng_given = brng is not None - if (brng): + if brng_given: # Parse before the lock to avoid warn brng_token, stream_id = _parse_brng_argument(brng) @@ -1808,7 +1820,7 @@ cdef class _MKLRandomState: obj = obj.astype("uint32", casting="unsafe", order="C") with self.lock: - if not brng: + if not brng_given: # Reads state->stream, which a concurrent seed can free. brng_token = irk_get_brng_and_stream_mkl( self.internal_state, &stream_id diff --git a/mkl_random/tests/test_random.py b/mkl_random/tests/test_random.py index 73c05f1..89050da 100644 --- a/mkl_random/tests/test_random.py +++ b/mkl_random/tests/test_random.py @@ -107,6 +107,35 @@ def test_non_deterministic_brng(): assert isinstance(v, int) +@pytest.mark.parametrize("brng", [11, 15, 99, -1, -100]) +def test_out_of_range_integer_brng_falls_back(brng): + with pytest.warns(UserWarning, match="not recognized"): + rs = rnd.MKLRandomState(1, brng=brng) + + expected = rnd.MKLRandomState(1, brng="MT19937").randint(0, 100, 8) + assert_equal(rs.randint(0, 100, 8), expected) + + +@pytest.mark.parametrize("brng_id,name", [(0, "MT19937"), (10, "ARS5")]) +def test_boundary_integer_brng_accepted(brng_id, name): + with assert_no_warnings(): + rs = rnd.MKLRandomState(1, brng=brng_id) + + expected = rnd.MKLRandomState(1, brng=name).randint(0, 100, 8) + assert_equal(rs.randint(0, 100, 8), expected) + + +def test_init_brng_none_uses_default(): + rs = rnd.MKLRandomState(1, brng=None) + expected = rnd.MKLRandomState(1, brng="MT19937").randint(0, 100, 8) + assert_equal(rs.randint(0, 100, 8), expected) + + rs = rnd.MKLRandomState(None, brng=None) + assert rs.get_state()[0] == "MT19937" + x = rs.random_sample(1000) + assert np.all((x >= 0) & (x < 1)) + + def test_binomial_n_zero(): zeros = np.zeros(2, dtype="int32") for p in [0, 0.5, 1]: