Migrate random `glue_lowering` to `overload` where easy by apmasell · Pull Request #8061 · numba/numba

apmasell · 2022-05-17T19:30:56Z

Convert all the glue_lowering to overload_method that is using
compile_internal to do basically exactly that.

gmarkall · 2022-05-18T08:08:26Z

gpuci run tests

apmasell · 2022-05-18T12:56:01Z

This is not ready for review.

Convert all the `glue_lowering` to `overload_method` that is using `compile_internal` to do basically exactly that.

sklam · 2022-05-31T23:24:21Z

/azp run

azure-pipelines · 2022-05-31T23:24:31Z

Azure Pipelines successfully started running 1 pipeline(s).

Add missing functions for handling random distributions in arrays

sklam

The type checking is a bit too restrictive. See comment below.

sklam · 2022-06-01T23:23:35Z

+    if (isinstance(ngood, types.Integer) and isinstance(nbad, types.Integer)
+       and isinstance(nsamples, types.Integer)):


This is overly restrictive because numpy is casting ngood and nbad to integers. In the old-style, the compiler auto emits the cast. You can replicate that behavior by returning 2-tuple of (sig, impl). See example at https://github.com/numba/numba/blob/main/numba/typed/dictobject.py#L728.

example:

import numpy as np from numba import njit args = [ (1, 2, 3), (10., 30., 4), ] @njit def foo(good, bad, sample): return np.random.hypergeometric(good, bad, sample) for good, bad, sample in args: print(foo(good, bad, sample)) print(foo.py_func(good, bad, sample))

Allow floating point types in hypergeometric distribution

sklam

For most of these three argument version, the first two argument typing doesn't match the two argument version (usually requires just types.Number). I also think you can let the two-arg version do the check by not checking in the three-arg version.

sklam · 2022-07-15T15:53:46Z

+            for idx in range(out.size):
+                out_flat[idx] = np.random.vonmises(mu, kappa)
+            return out
+        return _impl


Not exercised by tests

This is still need a test

Add checks for all random distributions array creation. Remove type checks for arguments in array creation functions that will be checked by the scalar versions. Also fixes bugs in several distributions.

sklam · 2022-07-19T21:02:16Z

            return out

-    elif isinstance(size, (types.UniTuple)) and isinstance(size.dtype, types.Integer):
+    elif isinstance(size, (types.UniTuple)) and isinstance(size.dtype,


Suggested change

elif isinstance(size, (types.UniTuple)) and isinstance(size.dtype,

elif isinstance(size, types.UniTuple) and isinstance(size.dtype,

sklam · 2022-07-19T21:27:39Z

+@overload(np.random.triangular)
+def triangular_impl(low, high, mode, size):
+    if isinstance(size, types.NoneType):
+        return lambda low, high, mode, _size: np.random.triangular(low, high,


Suggested change

return lambda low, high, mode, _size: np.random.triangular(low, high,

return lambda low, high, mode, size: np.random.triangular(low, high,

@overload requires the parameters to match exactly for the returned impl and the decorated func; otherwise you get error like:

Rejected as the implementation raised a specific error: InternalError: Typing and implementation arguments differ in argument names. Typing signature: (low, high, mode, size) Implementation signature: (low, high, mode, _size) Difference: {<Parameter "_size">, <Parameter "size">}

Also, the func(..., size=None) path is not exercised by the tests.

sklam · 2022-07-19T21:28:05Z

+@overload(np.random.gamma)
+def gamma_impl(alpha, beta, size):
+    if isinstance(size, types.NoneType):
+        return lambda alpha, beta, _size: np.random.gamma(alpha, beta)


Suggested change

return lambda alpha, beta, _size: np.random.gamma(alpha, beta)

return lambda alpha, beta, size: np.random.gamma(alpha, beta)

sklam · 2022-07-19T21:28:15Z

+@overload(np.random.standard_gamma)
+def standard_gamma_impl(alpha, size):
+    if isinstance(size, types.NoneType):
+        return lambda alpha, _size: np.random.standard_gamma(alpha)


Suggested change

return lambda alpha, _size: np.random.standard_gamma(alpha)

return lambda alpha, size: np.random.standard_gamma(alpha)

sklam · 2022-07-19T21:28:25Z

+@overload(np.random.beta)
+def beta_impl(alpha, beta, size):
+    if isinstance(size, types.NoneType):
+        return lambda alpha, beta, _size: np.random.beta(alpha, beta)


Suggested change

return lambda alpha, beta, _size: np.random.beta(alpha, beta)

return lambda alpha, beta, size: np.random.beta(alpha, beta)

sklam · 2022-07-19T21:31:36Z

+@overload(np.random.rayleigh)
+def rayleigh_impl2(mode, size):
+    if isinstance(size, types.NoneType):
+        return lambda mode, _size: np.random.rayleigh(mode)


Suggested change

return lambda mode, _size: np.random.rayleigh(mode)

return lambda mode, size: np.random.rayleigh(mode)

sklam · 2022-07-19T21:31:44Z

+@overload(np.random.standard_cauchy)
+def standard_cauchy_impl(size):
+    if isinstance(size, types.NoneType):
+        return lambda p, _size: np.random.standard_cauchy(p)


Suggested change

return lambda p, _size: np.random.standard_cauchy(p)

return lambda p, size: np.random.standard_cauchy(p)

sklam · 2022-07-19T21:31:55Z

+@overload(np.random.standard_t)
+def standard_t_impl2(df, size):
+    if isinstance(size, types.NoneType):
+        return lambda p, _size: np.random.standard_t(p)


Suggested change

return lambda p, _size: np.random.standard_t(p)

return lambda p, size: np.random.standard_t(p)

sklam · 2022-07-19T21:32:03Z

+@overload(np.random.wald)
+def wald_impl2(mean, scale, size):
+    if isinstance(size, types.NoneType):
+        return lambda mean, scale, _size: np.random.wald(mean, scale)


Suggested change

return lambda mean, scale, _size: np.random.wald(mean, scale)

return lambda mean, scale, size: np.random.wald(mean, scale)

sklam · 2022-07-19T21:32:11Z

+@overload(np.random.zipf)
+def zipf_impl(a, size):
+    if isinstance(size, types.NoneType):
+        return lambda a, _size: np.random.zipf(a)


Suggested change

return lambda a, _size: np.random.zipf(a)

return lambda a, size: np.random.zipf(a)

Correct parameters names to match main function's parameter names. Add checks for `size=None` to get a scalar value from random distributions (except for unmigrated ones).

The gamma distribution matches the CPython implementation, not the NumPy one, so create a special test that compares them appropriately when generating an array.

Make the logseries result be 64-bit rather than pointersized

sklam

these impl need to be exercised

This adds test for the remaining array implementations and also a new harness for ones that use the gamma distribution that won't match the Python NumPy version.

sklam · 2022-08-03T21:29:48Z

+            np_prod = n * p
+            bound = min(n, np_prod + 10.0 * math.sqrt(np_prod * q + 1))
+
+            finished = False


this variable is dead even in the old code

sklam

Two last comments:

Include test for vonmises random array. This is another one where the CPython algorithm is used instead of the NumPy one.

sklam

Thanks for the patch!

apmasell requested review from esc, sklam and stuartarchibald as code owners May 17, 2022 19:30

apmasell added the 2 - In Progress label May 17, 2022

Migrate random glue_lowering to overload where easy

bea6f65

Convert all the `glue_lowering` to `overload_method` that is using `compile_internal` to do basically exactly that.

apmasell changed the title ~~Migrate glue_lowering to overload_method where easy~~ Migrate random glue_lowering to overload where easy May 30, 2022

apmasell added 3 - Ready for Review and removed 2 - In Progress labels May 30, 2022

sklam reviewed May 31, 2022

View reviewed changes

Comment thread numba/core/typing/randomdecl.py

Comment thread numba/cpython/randomimpl.py Outdated

Fix missing array overrides

ba597b0

Add missing functions for handling random distributions in arrays

sklam reviewed Jun 1, 2022

View reviewed changes

sklam added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Jun 1, 2022

Relax types on hypergeometric

189c9ae

Allow floating point types in hypergeometric distribution

apmasell added 3 - Ready for Review and removed 4 - Waiting on author Waiting for author to respond to review labels Jun 2, 2022

sklam self-assigned this Jul 12, 2022

sklam added this to the Numba 0.57 RC milestone Jul 12, 2022

sklam reviewed Jul 15, 2022

View reviewed changes

Comment thread numba/cpython/randomimpl.py

Add missing tests and remove unncessary type checks

fe6fd8e

Add checks for all random distributions array creation. Remove type checks for arguments in array creation functions that will be checked by the scalar versions. Also fixes bugs in several distributions.

This was referenced Jul 18, 2022

Fix issue 8161 #8250

Merged

meta-issue: migrate @glue_* functions to @overload #8254

Closed

sklam reviewed Jul 19, 2022

View reviewed changes

apmasell added 2 commits July 20, 2022 13:31

Fix parameter names and test size=None

4fa48fa

Correct parameters names to match main function's parameter names. Add checks for `size=None` to get a scalar value from random distributions (except for unmigrated ones).

Create special unit tests for gamma distribution over array

c100b2f

The gamma distribution matches the CPython implementation, not the NumPy one, so create a special test that compares them appropriately when generating an array.

Ensure logseries works on 32-bit

f8684a9

Make the logseries result be 64-bit rather than pointersized

sklam reviewed Jul 26, 2022

View reviewed changes

Comment thread numba/cpython/randomimpl.py

Comment thread numba/cpython/randomimpl.py

Comment thread numba/cpython/randomimpl.py

Comment thread numba/cpython/randomimpl.py

Comment thread numba/cpython/randomimpl.py

sklam added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Jul 26, 2022

Add test for remaining random array implementations

74bfeae

This adds test for the remaining array implementations and also a new harness for ones that use the gamma distribution that won't match the Python NumPy version.

apmasell added 3 - Ready for Review 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jul 29, 2022

sklam reviewed Aug 3, 2022

View reviewed changes

sklam added the 4 - Waiting on author Waiting for author to respond to review label Aug 3, 2022

Add test for vonmises random array

f5c5b8f

Include test for vonmises random array. This is another one where the CPython algorithm is used instead of the NumPy one.

apmasell removed the 4 - Waiting on author Waiting for author to respond to review label Aug 4, 2022

sklam approved these changes Aug 17, 2022

View reviewed changes

sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 3 - Ready for Review labels Aug 17, 2022

sklam merged commit cf1d297 into numba:main Aug 17, 2022

apmasell deleted the unglue_easy branch August 26, 2022 18:28

stuartarchibald mentioned this pull request Jan 25, 2023

[DO NOT MERGE] Temp/pr8061 #8125

Closed

esc mentioned this pull request Jul 4, 2023

Numpy Gamma distribution not working in Numba 0.57+ #9053

Closed

		if (isinstance(ngood, types.Integer) and isinstance(nbad, types.Integer)
		and isinstance(nsamples, types.Integer)):

	elif isinstance(size, (types.UniTuple)) and isinstance(size.dtype,
	elif isinstance(size, types.UniTuple) and isinstance(size.dtype,

	return lambda low, high, mode, _size: np.random.triangular(low, high,
	return lambda low, high, mode, size: np.random.triangular(low, high,

	return lambda alpha, beta, _size: np.random.gamma(alpha, beta)
	return lambda alpha, beta, size: np.random.gamma(alpha, beta)

	return lambda alpha, _size: np.random.standard_gamma(alpha)
	return lambda alpha, size: np.random.standard_gamma(alpha)

	return lambda mode, _size: np.random.rayleigh(mode)
	return lambda mode, size: np.random.rayleigh(mode)

	return lambda p, _size: np.random.standard_cauchy(p)
	return lambda p, size: np.random.standard_cauchy(p)

	return lambda mean, scale, _size: np.random.wald(mean, scale)
	return lambda mean, scale, size: np.random.wald(mean, scale)

	return lambda a, _size: np.random.zipf(a)
	return lambda a, size: np.random.zipf(a)

Conversation

apmasell commented May 17, 2022

Uh oh!

gmarkall commented May 18, 2022

Uh oh!

apmasell commented May 18, 2022

Uh oh!

Uh oh!

Uh oh!

sklam commented May 31, 2022

Uh oh!

azure-pipelines Bot commented May 31, 2022

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

sklam left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants