From 6531049fb0160594f9f05566abde478aa08e7142 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Tue, 6 Oct 2026 20:04:01 +0200 Subject: [PATCH 1/2] fix: validate the sampling method and reject an empty mean in multinormal_cholesky choose_method returned any value that compared equal to a method id, so np.int64(0), np.int64(2), False and 1.0 passed through unchanged. The callers then compare the result with `is`, so those values were routed to the final else branch (BoxMuller) instead of the method they named. Unrecognized values, such as a misspelled name, were mapped to the default method without any notice. - choose_method now accepts strings from the alias table and integers (including NumPy integers) matching an id, and returns the module-level constant so the `is` comparisons keep working. - bool and float, which compare equal to the ids but are not valid specifications, and any other unrecognized value now emit a UserWarning and fall back to the function's default, like an unrecognized brng does. - multinormal_cholesky raises ValueError for an empty mean instead of ZeroDivisionError. - test_randomdist_lognormal asked lognormal for "Box-Muller2", which it does not support and silently treated as ICDF; it now asserts the warning. --- CHANGELOG.md | 3 ++ mkl_random/mklrand.pyx | 47 +++++++++++++++++++++------ mkl_random/tests/test_random.py | 57 +++++++++++++++++++++++++++++++-- 3 files changed, 94 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ecf2d23..1304c83 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Added an [ASV](https://asv.readthedocs.io/en/stable/) benchmark suite under `benchmarks/` and a `benchmark` optional dependency group [gh-184](https://github.com/IntelPython/mkl_random/pull/184) ### Changed +* Unrecognized `method` values (e.g. a misspelled name, `None`, `bool`, `float`) now emit a `UserWarning` before falling back to the function's default method, instead of falling back silently [gh-189](https://github.com/IntelPython/mkl_random/pull/189) * Sped up `normal`, `uniform`, `exponential`, `laplace`, `gumbel`, `logistic`, `rayleigh` and `lognormal` for array-valued parameters. Seeded results change for these array paths; scalar paths are unchanged [gh-171](https://github.com/IntelPython/mkl_random/pull/171) * Array parameters for these distributions must broadcast to the requested `size` without adding dimensions; previously accepted mismatches now raise `ValueError` [gh-171](https://github.com/IntelPython/mkl_random/pull/171) * `uniform` with array-valued bounds may return `high` due to floating-point rounding [gh-171](https://github.com/IntelPython/mkl_random/pull/171) @@ -27,6 +28,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Reduced the memory use and sped up integer generation with `WH`, `MCG31`, `R250` and `MRG32K3A`, which lack `viRngUniformBits` support [gh-178](https://github.com/IntelPython/mkl_random/pull/178) ### Fixed +* Fixed `multinormal_cholesky` and the other functions taking `method` routing integer-like ids such as `np.int64(0)`, `np.int64(2)` or `False` to `BoxMuller`; NumPy integers now select the method they name [gh-189](https://github.com/IntelPython/mkl_random/pull/189) +* Fixed `multinormal_cholesky` raising `ZeroDivisionError` for an empty `mean`; it now raises `ValueError` [gh-189](https://github.com/IntelPython/mkl_random/pull/189) * 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) diff --git a/mkl_random/mklrand.pyx b/mkl_random/mklrand.pyx index 959b7bf..e9d1e9e 100644 --- a/mkl_random/mklrand.pyx +++ b/mkl_random/mklrand.pyx @@ -1650,18 +1650,43 @@ _method_alias_dict_poisson = { } +_method_names = { + ICDF: "ICDF", + BOXMULLER: "BoxMuller", + BOXMULLER2: "BoxMuller2", + POISNORM: "POISNORM", + PTPE: "PTPE", +} + + def choose_method(method, mlist, alias_dict = None): - if (method not in mlist): - if (alias_dict is None) or (not isinstance(alias_dict, dict)): - # issue warning - return mlist[0] + """ + Resolve ``method`` to one of the module-level method constants in + ``mlist``. Callers compare the result with ``is``, so the constant itself + is returned, never an equal-valued object such as ``np.int64(2)``. + Unrecognized values, including ``bool`` and ``float``, which compare + equal to the integer constants, warn and fall back to ``mlist[0]``. + """ + if isinstance(method, str): + if isinstance(alias_dict, dict) and method in alias_dict: + return alias_dict[method] + elif not isinstance(method, (bool, np.bool_)): + try: + index = operator.index(method) + except TypeError: + pass else: - if method not in alias_dict.keys(): - return mlist[0] - else: - return alias_dict[method] - else: - return method + for candidate in mlist: + if candidate == index: + return candidate + + default = mlist[0] + warnings.warn( + f"The sampling method {method!r} is not recognized. " + f"\"{_method_names[default]}\" will be used instead", + UserWarning + ) + return default _brng_dict = { @@ -7409,6 +7434,8 @@ cdef class MKLRandomState(_MKLRandomState): if marr.ndim != 1: raise ValueError("mean must be 1 dimensional") dim = marr.shape[0] + if dim < 1: + raise ValueError("mean must have at least one element") if (tarr.ndim == 2): storage_mode = MATRIX if (tarr.shape[0] != tarr.shape[1]): diff --git a/mkl_random/tests/test_random.py b/mkl_random/tests/test_random.py index 89050da..d26e884 100644 --- a/mkl_random/tests/test_random.py +++ b/mkl_random/tests/test_random.py @@ -1001,9 +1001,11 @@ def test_randomdist_lognormal(randomdist): ] ) np.testing.assert_allclose(actual, desired, atol=1e-6, rtol=1e-10) - actual = rnd.lognormal( - mean=0.123456789, sigma=2.0, size=(3, 2), method="Box-Muller2" - ) + # lognormal only supports ICDF and BoxMuller, so this falls back to ICDF + with pytest.warns(UserWarning, match="not recognized"): + actual = rnd.lognormal( + mean=0.123456789, sigma=2.0, size=(3, 2), method="Box-Muller2" + ) desired = np.array( [ [0.2585388231094821, 0.43734953048924663], @@ -1119,6 +1121,55 @@ def test_randomdist_multinormal_cholesky(randomdist): np.testing.assert_allclose(actual, desired, atol=1e-10, rtol=1e-10) +@pytest.mark.parametrize("ch", [np.zeros((0, 0)), np.zeros(0)]) +def test_multinormal_cholesky_empty_mean(ch): + with pytest.raises(ValueError, match="at least one element"): + rnd.MKLRandomState(123).multinormal_cholesky(np.array([]), ch, size=3) + + +@pytest.mark.parametrize("method", ["ICDF", "BoxMuller", "BoxMuller2"]) +@pytest.mark.parametrize("to_int", [int, np.int64, np.uint8]) +def test_multinormal_cholesky_integer_method(method, to_int): + # Integer-like method ids must select the same method as the name. + ids = {"ICDF": 0, "BoxMuller": 1, "BoxMuller2": 2} + mean = np.array([0.1, -0.2]) + chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) + expected = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=method + ) + with warnings.catch_warnings(): + warnings.simplefilter("error") + actual = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=to_int(ids[method]) + ) + np.testing.assert_array_equal(actual, expected) + + +@pytest.mark.parametrize( + "method", ["bogus", 7, -1, 1.0, None, False, True, [1]] +) +def test_unrecognized_method_warns_and_uses_default(method): + # bool and float compare equal to the integer method ids, but are not + # valid method specifications. + mean = np.array([0.1, -0.2]) + chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) + expected = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method="ICDF" + ) + with pytest.warns(UserWarning, match="not recognized"): + actual = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=method + ) + np.testing.assert_array_equal(actual, expected) + + +def test_unrecognized_method_uses_each_functions_default(): + expected = rnd.MKLRandomState(123).poisson(3.0, size=8, method="POISNORM") + with pytest.warns(UserWarning, match="POISNORM"): + actual = rnd.MKLRandomState(123).poisson(3.0, size=8, method="bogus") + np.testing.assert_array_equal(actual, expected) + + def test_randomdist_negative_binomial(randomdist): rnd.seed(randomdist.seed, brng=randomdist.brng) actual = rnd.negative_binomial(n=100, p=0.12345, size=(3, 2)) From 8f8cb49780ed69e8a4282c4e2c431625e5c26a98 Mon Sep 17 00:00:00 2001 From: Anton Volkov Date: Tue, 6 Oct 2026 20:05:26 +0200 Subject: [PATCH 2/2] test: move the method and empty-mean tests away from the multinormal tests Place them next to test_normal_family_array_methods so they do not touch the same lines as other pull requests adding multinormal_cholesky tests. --- mkl_random/tests/test_random.py | 98 ++++++++++++++++----------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/mkl_random/tests/test_random.py b/mkl_random/tests/test_random.py index d26e884..8a55c0c 100644 --- a/mkl_random/tests/test_random.py +++ b/mkl_random/tests/test_random.py @@ -1121,55 +1121,6 @@ def test_randomdist_multinormal_cholesky(randomdist): np.testing.assert_allclose(actual, desired, atol=1e-10, rtol=1e-10) -@pytest.mark.parametrize("ch", [np.zeros((0, 0)), np.zeros(0)]) -def test_multinormal_cholesky_empty_mean(ch): - with pytest.raises(ValueError, match="at least one element"): - rnd.MKLRandomState(123).multinormal_cholesky(np.array([]), ch, size=3) - - -@pytest.mark.parametrize("method", ["ICDF", "BoxMuller", "BoxMuller2"]) -@pytest.mark.parametrize("to_int", [int, np.int64, np.uint8]) -def test_multinormal_cholesky_integer_method(method, to_int): - # Integer-like method ids must select the same method as the name. - ids = {"ICDF": 0, "BoxMuller": 1, "BoxMuller2": 2} - mean = np.array([0.1, -0.2]) - chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) - expected = rnd.MKLRandomState(123).multinormal_cholesky( - mean, chol_mat, size=8, method=method - ) - with warnings.catch_warnings(): - warnings.simplefilter("error") - actual = rnd.MKLRandomState(123).multinormal_cholesky( - mean, chol_mat, size=8, method=to_int(ids[method]) - ) - np.testing.assert_array_equal(actual, expected) - - -@pytest.mark.parametrize( - "method", ["bogus", 7, -1, 1.0, None, False, True, [1]] -) -def test_unrecognized_method_warns_and_uses_default(method): - # bool and float compare equal to the integer method ids, but are not - # valid method specifications. - mean = np.array([0.1, -0.2]) - chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) - expected = rnd.MKLRandomState(123).multinormal_cholesky( - mean, chol_mat, size=8, method="ICDF" - ) - with pytest.warns(UserWarning, match="not recognized"): - actual = rnd.MKLRandomState(123).multinormal_cholesky( - mean, chol_mat, size=8, method=method - ) - np.testing.assert_array_equal(actual, expected) - - -def test_unrecognized_method_uses_each_functions_default(): - expected = rnd.MKLRandomState(123).poisson(3.0, size=8, method="POISNORM") - with pytest.warns(UserWarning, match="POISNORM"): - actual = rnd.MKLRandomState(123).poisson(3.0, size=8, method="bogus") - np.testing.assert_array_equal(actual, expected) - - def test_randomdist_negative_binomial(randomdist): rnd.seed(randomdist.seed, brng=randomdist.brng) actual = rnd.negative_binomial(n=100, p=0.12345, size=(3, 2)) @@ -1541,6 +1492,55 @@ def test_normal_family_array_methods(name, method, normal_method, size): ) +@pytest.mark.parametrize("ch", [np.zeros((0, 0)), np.zeros(0)]) +def test_multinormal_cholesky_empty_mean(ch): + with pytest.raises(ValueError, match="at least one element"): + rnd.MKLRandomState(123).multinormal_cholesky(np.array([]), ch, size=3) + + +@pytest.mark.parametrize("method", ["ICDF", "BoxMuller", "BoxMuller2"]) +@pytest.mark.parametrize("to_int", [int, np.int64, np.uint8]) +def test_multinormal_cholesky_integer_method(method, to_int): + # Integer-like method ids must select the same method as the name. + ids = {"ICDF": 0, "BoxMuller": 1, "BoxMuller2": 2} + mean = np.array([0.1, -0.2]) + chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) + expected = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=method + ) + with warnings.catch_warnings(): + warnings.simplefilter("error") + actual = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=to_int(ids[method]) + ) + np.testing.assert_array_equal(actual, expected) + + +@pytest.mark.parametrize( + "method", ["bogus", 7, -1, 1.0, None, False, True, [1]] +) +def test_unrecognized_method_warns_and_uses_default(method): + # bool and float compare equal to the integer method ids, but are not + # valid method specifications. + mean = np.array([0.1, -0.2]) + chol_mat = np.array([[1.0, 0.0], [-0.5, 1.0]]) + expected = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method="ICDF" + ) + with pytest.warns(UserWarning, match="not recognized"): + actual = rnd.MKLRandomState(123).multinormal_cholesky( + mean, chol_mat, size=8, method=method + ) + np.testing.assert_array_equal(actual, expected) + + +def test_unrecognized_method_uses_each_functions_default(): + expected = rnd.MKLRandomState(123).poisson(3.0, size=8, method="POISNORM") + with pytest.warns(UserWarning, match="POISNORM"): + actual = rnd.MKLRandomState(123).poisson(3.0, size=8, method="bogus") + np.testing.assert_array_equal(actual, expected) + + @pytest.mark.parametrize( "name,draw,p", [