cunumeric.random - sort and shuffle-based operations#559
cunumeric.random - sort and shuffle-based operations#559fduguet-nv wants to merge 8 commits intonv-legate:branch-24.03from
Conversation
| """ | ||
|
|
||
| if isinstance(a, int) and a <= 0: | ||
| raise ValueError |
There was a problem hiding this comment.
Numpy includes a specific message for the exception
Input In [3], in <cell line: 1>()
----> 1 np.random.choice(a, -1)
File mtrand.pyx:962, in numpy.random.mtrand.RandomState.choice()
File mtrand.pyx:748, in numpy.random.mtrand.RandomState.randint()
File _bounded_integers.pyx:1256, in numpy.random._bounded_integers._rand_int64()
ValueError: negative dimensions are not allowed| if isinstance(a, int) and a <= 0: | ||
| raise ValueError | ||
| if not isinstance(size, int): | ||
| raise ValueError |
There was a problem hiding this comment.
Numpy lets a TypeError go through in this situation
In [4]: np.random.choice(a, 2, p=3.4)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
Input In [4], in <cell line: 1>()
----> 1 np.random.choice(a, 2, p=3.4)
File mtrand.pyx:918, in numpy.random.mtrand.RandomState.choice()
TypeError: object of type 'float' has no len()we could explicitly raise our own TypeError here if it doesn't arise naturally in our codepaths, but we should match exception type with Numpy in any case.
| p : 1-D array-like : | ||
| The probabilities associated with each entry in ``a``. | ||
| If not given the sample assumes a uniform distribution over all | ||
| entries in ``a``. |
There was a problem hiding this comment.
optional params need "optional" and also to state the default values. c.f. numpy docstring:
https://numpy.org/doc/stable/reference/random/generated/numpy.random.choice.html
| return cunumeric.random.randint(0, a, size) | ||
| else: | ||
| if a < size: | ||
| raise ValueError |
There was a problem hiding this comment.
Numpy has a descriptive error message
In [9]: np.random.choice(5, size=8, replace=False)
---------------------------------------------------------------------------
ValueError Traceback (most recent call last)
Input In [9], in <cell line: 1>()
----> 1 np.random.choice(5, size=8, replace=False)
File mtrand.pyx:965, in numpy.random.mtrand.RandomState.choice()
ValueError: Cannot take a larger sample than population when 'replace=False'| return a[indices] | ||
| else: | ||
| if len(a) < size: | ||
| raise ValueError |
There was a problem hiding this comment.
same as above:
ValueError: Cannot take a larger sample than population when 'replace=False'
| # check if p is a probability distribution | ||
| if abs(cump[-1] - 1.0) > len(p) * np.finfo(p.dtype).eps: | ||
| # does not sum up to 1 | ||
| raise ValueError |
There was a problem hiding this comment.
From Numpy:
ValueError: probabilities do not sum to 1
| return cunumeric.random.permutation(a)[:size] | ||
| else: | ||
| if not replace: | ||
| raise ValueError |
There was a problem hiding this comment.
| raise ValueError | |
| raise ValueError("cunumeric.random.choice does not currently support passing 'p' when 'replace=False'") |
|
@fduguet-nv test changes look good, I did poke into comparing the exception conditions in more detail, and I think they just need tweaks to match Numpy more closely |
2 participants
Implementation of: