Repository: numpy/numpy
Pull request: MAINT: Large overhead in some random functions
Fixes https://github.com/numpy/numpy/issues/15460

- slow calls to np.dtype.name replaced with np.dtype,
- mtrand.pyx and _generator.pyx updated
- test `_test_warns_byteorder_` updated



Problem statement:
BUG: Large overhead in some random functions
This issue is extracted from gitter (by @andyfaff – I saw it only randomly, so creating this before we forget).

Some random generator functions such as `random()` have a huge overhead compared to their legacy `RandomState` versions:
```
rs = np.random.RandomState()
rg = np.random.default_rng()

%timeit rs.random()                                       
# 432 ns ± 9.18 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

%timeit rg.random()                                       
# 5.19 µs ± 61.6 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
```

The reason for this is the support of the `dtype=` keyword argument. A secondary reason (and maybe second speed issue) is that `np.dtype.name` is a *very* slow operations (it could plausibly be cached).

However, the solution here will be to simply avoid the whole `np.dtype(dtype).name` construct as much as possible and maybe adding a fastpath for the case when `dtype` is not used. `np.dtype(dtype).type is np.float64` may be a solution, or `dtype is np.float64 or np.dtype(dtype) is np.float64` to speed up the default branch.

---

Checking that we have benchmarks – or adding simple small array ones in a first commit – for this would be good when this is fixed.

Hints from issue discussion:
> avoid the whole np.dtype(dtype).name construct as much as possible

Agreed, hopefully the slowness is enough to motivate people not to use it ;)
Is a similar speedup possible for `Generator.uniform`, as well as `Generator.random`?. It seems that they're probably going to be doing exactly the same, except the former does the optional scaling.

```
>>> %timeit rs.random()
308 ns ± 7.4 ns per loop (mean ± std. dev. of 7 runs, 1000000 loops each)

>>> %timeit rg.uniform()
2.11 µs ± 73.4 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

>>> %timeit rg.random()
3.37 µs ± 85.8 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
```
We're only talking a couple of microseconds, but it's worth it if it's an easy win.
I consider it a speed bug, since the added overhead is a small factor larger than a typical (fairly simple) python function call (and almost a factor of 10 of what it should be).
This is probably a fairly straight forward issue to solve, so PRs welcome.
I will try to speed it up ;) WIP
@przemb you may find the comments starting [here](https://github.com/numpy/numpy/pull/15463#pullrequestreview-351965452) from gh-15463 useful

Commit message: MAINT: Large overhead in some random functions #15460

slow calls to np.dtype.name replaced with np.dtype,
mtrand.pyx and _generator.pyx updated,
test test_warns_byteorder updated

before:
%timeit rs.random(): 520 ns ± 33.1 ns per loop
%timeit rg.random(): 6.36 µs ± 222 ns per loop

after:
%timeit rs.random(): 453 ns ± 6.95 ns per loop
%timeit rg.random(): 594 ns ± 9.66 ns per loop

Patch to review:
diff --git a/numpy/random/_generator.pyx b/numpy/random/_generator.pyx
index 32eda25f793d..c7ba4c534fe7 100644
--- a/numpy/random/_generator.pyx
+++ b/numpy/random/_generator.pyx
@@ -17,7 +17,6 @@ from libc.stdint cimport (uint8_t, uint16_t, uint32_t, uint64_t,
 from ._bounded_integers cimport (_rand_bool, _rand_int32, _rand_int64,
          _rand_int16, _rand_int8, _rand_uint64, _rand_uint32, _rand_uint16,
          _rand_uint8, _gen_mask)
-from ._bounded_integers import _integers_types
 from ._pcg64 import PCG64
 from numpy.random cimport bitgen_t
 from ._common cimport (POISSON_LAM_MAX, CONS_POSITIVE, CONS_NONE,
@@ -312,13 +311,13 @@ cdef class Generator:
 
         """
         cdef double temp
-        key = np.dtype(dtype).name
-        if key == 'float64':
+        _dtype = np.dtype(dtype)
+        if _dtype == np.float64:
             return double_fill(&random_standard_uniform_fill, &self._bitgen, size, self.lock, out)
-        elif key == 'float32':
+        elif _dtype == np.float32:
             return float_fill(&random_standard_uniform_fill_f, &self._bitgen, size, self.lock, out)
         else:
-            raise TypeError('Unsupported dtype "%s" for random' % key)
+            raise TypeError('Unsupported dtype %r for random' % _dtype)
 
     def beta(self, a, b, size=None):
         """
@@ -454,20 +453,20 @@ cdef class Generator:
         >>> n = np.random.default_rng().standard_exponential((3, 8000))
 
         """
-        key = np.dtype(dtype).name
-        if key == 'float64':
+        _dtype = np.dtype(dtype)
+        if _dtype == np.float64:
             if method == u'zig':
                 return double_fill(&random_standard_exponential_fill, &self._bitgen, size, self.lock, out)
             else:
                 return double_fill(&random_standard_exponential_inv_fill, &self._bitgen, size, self.lock, out)
-        elif key == 'float32':
+        elif _dtype == np.float32:
             if method == u'zig':
                 return float_fill(&random_standard_exponential_fill_f, &self._bitgen, size, self.lock, out)
             else:
                 return float_fill(&random_standard_exponential_inv_fill_f, &self._bitgen, size, self.lock, out)
         else:
-            raise TypeError('Unsupported dtype "%s" for standard_exponential'
-                            % key)
+            raise TypeError('Unsupported dtype %r for standard_exponential'
+                            % _dtype)
 
     def integers(self, low, high=None, size=None, dtype=np.int64, endpoint=False):
         """
@@ -559,39 +558,39 @@ cdef class Generator:
             high = low
             low = 0
 
-        dt = np.dtype(dtype)
-        key = dt.name
-        if key not in _integers_types:
-            raise TypeError('Unsupported dtype "%s" for integers' % key)
-        if not dt.isnative:
-            raise ValueError('Providing a dtype with a non-native byteorder '
-                             'is not supported. If you require '
-                             'platform-independent byteorder, call byteswap '
-                             'when required.')
+        _dtype = np.dtype(dtype)
 
         # Implementation detail: the old API used a masked method to generate
         # bounded uniform integers. Lemire's method is preferable since it is
         # faster. randomgen allows a choice, we will always use the faster one.
         cdef bint _masked = False
 
-        if key == 'int32':
+        if _dtype == np.int32:
             ret = _rand_int32(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'int64':
+        elif _dtype == np.int64:
             ret = _rand_int64(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'int16':
+        elif _dtype == np.int16:
             ret = _rand_int16(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'int8':
+        elif _dtype == np.int8:
             ret = _rand_int8(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'uint64':
+        elif _dtype == np.uint64:
             ret = _rand_uint64(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'uint32':
+        elif _dtype == np.uint32:
             ret = _rand_uint32(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'uint16':
+        elif _dtype == np.uint16:
             ret = _rand_uint16(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'uint8':
+        elif _dtype == np.uint8:
             ret = _rand_uint8(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
-        elif key == 'bool':
+        elif _dtype == np.bool_:
             ret = _rand_bool(low, high, size, _masked, endpoint, &self._bitgen, self.lock)
+        elif not _dtype.isnative:
+            raise ValueError('Providing a dtype with a non-native byteorder '
+                             'is not supported. If you require '
+                             'platform-independent byteorder, call byteswap '
+                             'when required.')
+        else:
+            raise TypeError('Unsupported dtype %r for integers' % _dtype)
+
 
         if size is None and dtype in (bool, int, np.compat.long):
             if np.array(ret).shape == ():
@@ -1041,14 +1040,14 @@ cdef class Generator:
                [ 0.39924804,  4.68456316,  4.99394529,  4.84057254]])  # random
 
         """
-        key = np.dtype(dtype).name
-        if key == 'float64':
+        _dtype = np.dtype(dtype)
+        if _dtype == np.float64:
             return double_fill(&random_standard_normal_fill, &self._bitgen, size, self.lock, out)
-        elif key == 'float32':
+        elif _dtype == np.float32:
             return float_fill(&random_standard_normal_fill_f, &self._bitgen, size, self.lock, out)
 
         else:
-            raise TypeError('Unsupported dtype "%s" for standard_normal' % key)
+            raise TypeError('Unsupported dtype %r for standard_normal' % _dtype)
 
     def normal(self, loc=0.0, scale=1.0, size=None):
         """
@@ -1230,19 +1229,19 @@ cdef class Generator:
 
         """
         cdef void *func
-        key = np.dtype(dtype).name
-        if key == 'float64':
+        _dtype = np.dtype(dtype)
+        if _dtype == np.float64:
             return cont(&random_standard_gamma, &self._bitgen, size, self.lock, 1,
                         shape, 'shape', CONS_NON_NEGATIVE,
                         0.0, '', CONS_NONE,
                         0.0, '', CONS_NONE,
                         out)
-        if key == 'float32':
+        if _dtype == np.float32:
             return cont_f(&random_standard_gamma_f, &self._bitgen, size, self.lock,
                           shape, 'shape', CONS_NON_NEGATIVE,
                           out)
         else:
-            raise TypeError('Unsupported dtype "%s" for standard_gamma' % key)
+            raise TypeError('Unsupported dtype %r for standard_gamma' % _dtype)
 
     def gamma(self, shape, scale=1.0, size=None):
         """
diff --git a/numpy/random/mtrand.pyx b/numpy/random/mtrand.pyx
index a04026c0df5e..b38e8bbbe04e 100644
--- a/numpy/random/mtrand.pyx
+++ b/numpy/random/mtrand.pyx
@@ -15,7 +15,6 @@ from libc.stdint cimport int64_t, uint64_t
 from ._bounded_integers cimport (_rand_bool, _rand_int32, _rand_int64,
          _rand_int16, _rand_int8, _rand_uint64, _rand_uint32, _rand_uint16,
          _rand_uint8,)
-from ._bounded_integers import _integers_types
 from ._mt19937 import MT19937 as _MT19937
 from numpy.random cimport bitgen_t
 from ._common cimport (POISSON_LAM_MAX, CONS_POSITIVE, CONS_NONE,
@@ -724,17 +723,7 @@ cdef class RandomState:
             high = low
             low = 0
 
-        dt = np.dtype(dtype)
-        key = dt.name
-        if key not in _integers_types:
-            raise TypeError('Unsupported dtype "%s" for randint' % key)
-        if not dt.isnative:
-            # numpy 1.17.0, 2019-05-28
-            warnings.warn('Providing a dtype with a non-native byteorder is '
-                          'not supported. If you require platform-independent '
-                          'byteorder, call byteswap when required.\nIn future '
-                          'version, providing byteorder will raise a '
-                          'ValueError', DeprecationWarning)
+        _dtype = np.dtype(dtype)
 
         # Implementation detail: the use a masked method to generate
         # bounded uniform integers. Lemire's method is preferable since it is
@@ -743,24 +732,31 @@ cdef class RandomState:
         cdef bint _masked = True
         cdef bint _endpoint = False
 
-        if key == 'int32':
+        if _dtype == np.int32:
             ret = _rand_int32(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int64':
+        elif _dtype == np.int64:
             ret = _rand_int64(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int16':
+        elif _dtype == np.int16:
             ret = _rand_int16(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int8':
+        elif _dtype == np.int8:
             ret = _rand_int8(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint64':
+        elif _dtype == np.uint64:
             ret = _rand_uint64(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint32':
+        elif _dtype == np.uint32:
             ret = _rand_uint32(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint16':
+        elif _dtype == np.uint16:
             ret = _rand_uint16(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint8':
+        elif _dtype == np.uint8:
             ret = _rand_uint8(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'bool':
+        elif _dtype == np.bool_:
             ret = _rand_bool(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
+        elif not _dtype.isnative:
+            raise ValueError('Providing a dtype with a non-native byteorder '
+                             'is not supported. If you require '
+                             'platform-independent byteorder, call byteswap '
+                             'when required.')
+        else:
+            raise TypeError('Unsupported dtype %r for randint' % _dtype)
 
         if size is None and dtype in (bool, int, np.compat.long):
             if np.array(ret).shape == ():
diff --git a/numpy/random/tests/test_randomstate_regression.py b/numpy/random/tests/test_randomstate_regression.py
index 1d8a0ed5a609..9f9728df249b 100644
--- a/numpy/random/tests/test_randomstate_regression.py
+++ b/numpy/random/tests/test_randomstate_regression.py
@@ -163,8 +163,7 @@ def __array__(self):
     def test_warns_byteorder(self):
         # GH 13159
         other_byteord_dt = '<i4' if sys.byteorder == 'big' else '>i4'
-        with pytest.deprecated_call(match='non-native byteorder is not'):
-            random.randint(0, 200, size=10, dtype=other_byteord_dt)
+        assert_raises(ValueError, random.randint, 0, 200, size=10, dtype=other_byteord_dt)
 
     def test_named_argument_initialization(self):
         # GH 13669


Code review comments:
diff_hunk: @@ -724,17 +723,7 @@ cdef class RandomState:
             high = low
             low = 0
 
-        dt = np.dtype(dtype)
-        key = dt.name
-        if key not in _integers_types:
-            raise TypeError('Unsupported dtype "%s" for randint' % key)
-        if not dt.isnative:
-            # numpy 1.17.0, 2019-05-28
-            warnings.warn('Providing a dtype with a non-native byteorder is '
path: numpy/random/mtrand.pyx
text: @user1:
I think you can keep this and add `dt = dt.newbyteorder()`

@author:
Ok, good idea.

diff_hunk: @@ -743,24 +732,31 @@ cdef class RandomState:
         cdef bint _masked = True
         cdef bint _endpoint = False
 
-        if key == 'int32':
+        if _dtype == np.int32:
             ret = _rand_int32(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int64':
+        elif _dtype == np.int64:
             ret = _rand_int64(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int16':
+        elif _dtype == np.int16:
             ret = _rand_int16(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'int8':
+        elif _dtype == np.int8:
             ret = _rand_int8(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint64':
+        elif _dtype == np.uint64:
             ret = _rand_uint64(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint32':
+        elif _dtype == np.uint32:
             ret = _rand_uint32(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint16':
+        elif _dtype == np.uint16:
             ret = _rand_uint16(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'uint8':
+        elif _dtype == np.uint8:
             ret = _rand_uint8(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
-        elif key == 'bool':
+        elif _dtype == np.bool_:
             ret = _rand_bool(low, high, size, _masked, _endpoint, &self._bitgen, self.lock)
+        elif not _dtype.isnative:
+            raise ValueError('Providing a dtype with a non-native byteorder '
+                             'is not supported. If you require '
+                             'platform-independent byteorder, call byteswap '
+                             'when required.')
path: numpy/random/mtrand.pyx
text: @user1:
This changes emitting a deprecation warning into an error, but according to our (still draft-status) [policy](https://numpy.org/neps/nep-0023-backwards-compatibility.html#policy) there should be at least a year between the start of the deprecation cycle until its removal..

@author:
In case of _mtrand.pyx_ a warning was thrown, but in _generator.pyx_ ValueError was raised. I changed it to keep it consistent. Thank you for the explanation, I will roll back my modification.