Skip to content

Support array_like low/high bounds for randint - #168

Open
vlad-perevezentsev wants to merge 18 commits into
masterfrom
randint_array_like_bounds
Open

Support array_like low/high bounds for randint#168
vlad-perevezentsev wants to merge 18 commits into
masterfrom
randint_array_like_bounds

Conversation

@vlad-perevezentsev

@vlad-perevezentsev vlad-perevezentsev commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

This PR adds array_like low/high support to randint to align with numpy.random.RandomState.randint

It adds new C routines irk_rand_<type>_broadcast for all 9 integer dtypes, drawing per-element integers over [low, high) with Lemire's multiply-shift (the same method as NumPy).
An earlier masked-rejection version benchmarked slower so it was replaced.

A mulhi helper returns the high half of the product because Lemire computes word × range which overflows the type so the 64-bit case uses a 32-bit schoolbook multiply.

It also groups all randint tests into TestRandint class and adds new ones.

@ndgrigorian

Copy link
Copy Markdown
Collaborator

@vlad-perevezentsev
We will want to apply to randint_untyped as well

Comment thread mkl_random/mklrand.pyx Outdated
Comment thread mkl_random/mklrand.pyx Outdated
@ndgrigorian

Copy link
Copy Markdown
Collaborator

@vlad-perevezentsev
Edge case issue:

In [5]: np.random.randint([3], [9], size=())
Out[5]: array(6)

In [6]: rand.randint([3], [9], size=())
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
Cell In[6], line 1
----> 1 rand.randint([3], [9], size=())

File mkl_random/mklrand.pyx:2455, in mkl_random.mklrand._MKLRandomState.randint()
-> 2455 'Could not get source, probably due dynamically evaluated source code.'

File mkl_random/mklrand.pyx:2302, in mkl_random.mklrand._MKLRandomState._randint_broadcast()
-> 2302 'Could not get source, probably due dynamically evaluated source code.'

File ~/miniforge3/envs/dpctl_dev/lib/python3.14t/site-packages/numpy/lib/_stride_tricks_impl.py:443, in broadcast_to(array, shape, subok)
    400 @array_function_dispatch(_broadcast_to_dispatcher, module='numpy')
    401 def broadcast_to(array, shape, subok=False):
    402     """Broadcast an array to a new shape.
    403
    404     Parameters
   (...)    441            [1, 2, 3]])
    442     """
--> 443     return _broadcast_to(array, shape, subok=subok, readonly=True)

File ~/miniforge3/envs/dpctl_dev/lib/python3.14t/site-packages/numpy/lib/_stride_tricks_impl.py:377, in _broadcast_to(array, shape, subok, readonly)
    375 array = np.array(array, copy=None, subok=subok)
    376 if not shape and array.shape:
--> 377     raise ValueError('cannot broadcast a non-scalar to a scalar array')
    378 if any(size < 0 for size in shape):
    379     raise ValueError('all elements of broadcast shape must be non-'
    380                      'negative')

ValueError: cannot broadcast a non-scalar to a scalar array

@ndgrigorian

Copy link
Copy Markdown
Collaborator

@vlad-perevezentsev
We get incorrect results (OOB) for smaller int types:

In [1]: import mkl_random as rand
<frozen importlib._bootstrap>:491: RuntimeWarning: The global interpreter lock (GIL) has been enabled to load module 'mkl_random.mklrand', which has not declared that it can run safely without the GIL. To override this behavior and keep the GIL disabled (at your own risk), run with PYTHON_GIL=0 or -Xgil=0.

In [2]: import numpy as np

In [3]: rand.randint([-5], [6], dtype=np.int8)
Out[3]: array([-119], dtype=int8)

In [4]: np.random.randint([-5], [6], dtype=np.int8)
Out[4]: array([-5], dtype=int8)

Comment thread mkl_random/src/mkl_distributions.cpp Outdated
Comment thread mkl_random/src/mkl_distributions.cpp
@antonwolfy antonwolfy added this to the 1.6.0 release milestone Sep 2, 2026
int err = 0;
npy_intp i = 0;

while (len > 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good at some point to experiment with chunking these loops rather than going with a single-pass. Can we add a TODO comment to remind of experimenting?

@ndgrigorian ndgrigorian left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM, in the follow-up we should fix the remaining types and also fix randint_untyped to align with the others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants