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..8a55c0c 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], @@ -1490,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", [