Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add triangular indices functions #4601

Merged
merged 11 commits into from
Oct 1, 2019
4 changes: 4 additions & 0 deletions docs/source/reference/numpysupported.rst
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,11 @@ The following top-level functions are supported:
* :func:`numpy.trapz` (only the 3 first arguments)
* :func:`numpy.tri` (only the 3 first arguments; third argument ``k`` must be an integer)
* :func:`numpy.tril` (second argument ``k`` must be an integer)
* :func:`numpy.tril_indices` (all arguments must be integer)
* :func:`numpy.tril_indices_from` (second argument ``k`` must be an integer)
* :func:`numpy.triu` (second argument ``k`` must be an integer)
* :func:`numpy.triu_indices` (all arguments must be integer)
* :func:`numpy.triu_indices_from` (second argument ``k`` must be an integer)
* :func:`numpy.unique` (only the first argument)
* :func:`numpy.vander`
* :func:`numpy.vstack`
Expand Down
54 changes: 54 additions & 0 deletions numba/targets/arraymath.py
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,33 @@ def np_tril_impl_multi(m, k=0):
else:
return np_tril_impl_multi

@overload(np.tril_indices)
def np_tril_indices(n, k=0, m=None):

# we require integer arguments, unlike numpy
esc marked this conversation as resolved.
Show resolved Hide resolved
if not isinstance(n, (int, types.Integer)):
raise TypeError('n must be an integer')
if not isinstance(k, (int, types.Integer)):
raise TypeError('k must be an integer')
if not (m is None or isinstance(m, (int, types.Integer))):
raise TypeError('m must be an integer')

def np_tril_indices_impl(n, k=0, m=None):
return np.nonzero(np.tri(n, m, k=k))
EPronovost marked this conversation as resolved.
Show resolved Hide resolved
return np_tril_indices_impl

@overload(np.tril_indices_from)
def np_tril_indices_from(arr, k=0):

# we require k to be integer, unlike numpy
if not isinstance(k, (int, types.Integer)):
raise TypeError('k must be an integer')

def np_tril_indices_from_impl(arr, k=0):
if arr.ndim != 2:
raise ValueError("input array must be 2-d")
return np.tril_indices(arr.shape[0], k=k, m=arr.shape[1])
return np_tril_indices_from_impl

@register_jitable
def np_triu_impl_2d(m, k=0):
Expand Down Expand Up @@ -1446,6 +1473,33 @@ def np_triu_impl_multi(m, k=0):
else:
return np_triu_impl_multi

@overload(np.triu_indices)
def np_triu_indices(n, k=0, m=None):

# we require integer arguments, unlike numpy
if not isinstance(n, (int, types.Integer)):
raise TypeError('n must be an integer')
if not isinstance(k, (int, types.Integer)):
raise TypeError('k must be an integer')
if not (m is None or isinstance(m, (int, types.Integer))):
raise TypeError('m must be an integer')

def np_triu_indices_impl(n, k=0, m=None):
return np.nonzero(1 - np.tri(n, m, k=k-1))
return np_triu_indices_impl

@overload(np.triu_indices_from)
def np_triu_indices_from(arr, k=0):

# we require k to be integer, unlike numpy
if not isinstance(k, (int, types.Integer)):
raise TypeError('k must be an integer')

def np_triu_indices_from_impl(arr, k=0):
if arr.ndim != 2:
raise ValueError("input array must be 2-d")
return np.triu_indices(arr.shape[0], k=k, m=arr.shape[1])
return np_triu_indices_from_impl

def _prepare_array(arr):
pass
Expand Down
148 changes: 147 additions & 1 deletion numba/tests/test_np_functions.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,41 @@ def tril_m(m):
def tril_m_k(m, k=0):
return np.tril(m, k)

def tril_indices_n(n):
return np.tril_indices(n)

def tril_indices_n_k(n, k=0):
return np.tril_indices(n, k)

def tril_indices_n_k_m(n, k=0, m=None):
return np.tril_indices(n, k, m)

esc marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

There is one last corner case that you should address:

def tril_indices_n_k_m(n, k=0, m=None):
    return np.tril_indices(n, k=k, m=m)

I.e. supplying the arguments as keyword arguments. I recently discovered that this might lead to slightly different path and should be checked to be safe.

def tril_indices_from_arr(arr):
return np.tril_indices_from(arr)

def tril_indices_from_arr_k(arr, k=0):
return np.tril_indices_from(arr, k)

def triu_m(m):
return np.triu(m)


def triu_m_k(m, k=0):
return np.triu(m, k)

def triu_indices_n(n):
return np.triu_indices(n)

def triu_indices_n_k(n, k=0):
return np.triu_indices(n, k)

def triu_indices_n_k_m(n, k=0, m=None):
return np.triu_indices(n, k, m)

esc marked this conversation as resolved.
Show resolved Hide resolved
def triu_indices_from_arr(arr):
return np.triu_indices_from(arr)

def triu_indices_from_arr_k(arr, k=0):
return np.triu_indices_from(arr, k)

def vander(x, N=None, increasing=False):
return np.vander(x, N, increasing)
Expand Down Expand Up @@ -1066,20 +1093,139 @@ def _triangular_matrix_exceptions(self, pyfunc):
cfunc(a, k=1.5)
assert "k must be an integer" in str(raises.exception)

def _triangular_indices_tests_n(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

for n in range(10):
expected = pyfunc(n)
got = cfunc(n)
self.assertEqual(type(expected), type(got))
self.assertEqual(len(expected), len(got))
for e, g in zip(expected, got):
np.testing.assert_array_equal(e, g)

def _triangular_indices_tests_n_k(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

for n in range(10):
for k in range(-n - 1, n + 2):
expected = pyfunc(n, k)
got = cfunc(n, k)
self.assertEqual(type(expected), type(got))
self.assertEqual(len(expected), len(got))
for e, g in zip(expected, got):
np.testing.assert_array_equal(e, g)

def _triangular_indices_tests_n_k_m(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

for n in range(10):
for m in range(2 * n):
for k in range(-n - 1, n + 2):
expected = pyfunc(n, k, m)
got = cfunc(n, k, m)
self.assertEqual(type(expected), type(got))
self.assertEqual(len(expected), len(got))
for e, g in zip(expected, got):
np.testing.assert_array_equal(e, g)

esc marked this conversation as resolved.
Show resolved Hide resolved
def _triangular_indices_from_tests_arr(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

for dtype in [int, float, bool]:
for n in range(10):
for m in range(10):
Copy link
Contributor

Choose a reason for hiding this comment

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

itertools.product!?

Copy link
Member

Choose a reason for hiding this comment

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

sure

arr = np.ones((n, m), dtype)
Copy link
Contributor

Choose a reason for hiding this comment

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

this is just testing matrices that are full? given the alg, perhaps put in some zeros via a slice with step or similar?!

Copy link
Member

Choose a reason for hiding this comment

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

You mean randomly or with structure?

Copy link
Contributor

Choose a reason for hiding this comment

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

don't think it matters, whatever is convenient.

Copy link
Contributor

Choose a reason for hiding this comment

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

actually, ignore this, am remembering a different function!

expected = pyfunc(arr)
got = cfunc(arr)
self.assertEqual(type(expected), type(got))
self.assertEqual(len(expected), len(got))
for e, g in zip(expected, got):
np.testing.assert_array_equal(e, g)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd assume that the arrays returned would be int type so self.assertPreciseEqual perhaps could be used?

Copy link
Member

Choose a reason for hiding this comment

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

I tried this, and the strides turned out to be different:

FAIL: test_tril_indices_from (numba.tests.test_np_functions.TestNPFunctions)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/vhaenel/git/numba/numba/tests/test_np_functions.py", line 1288, in test_tril_indices_from
    self._triangular_indices_from_tests_arr(tril_indices_from_arr)
  File "/Users/vhaenel/git/numba/numba/tests/test_np_functions.py", line 1221, in _triangular_indices_from_tests_arr
    self.assertPreciseEqual(e, g)
  File "/Users/vhaenel/git/numba/numba/tests/support.py", line 270, in assertPreciseEqual
    self.fail("when comparing %s and %s: %s" % (first, second, failure_msg))
AssertionError: when comparing [0 1] and [0 1]: Lists differ: [2.0] != [1.0]

First differing element 0:
2.0
1.0

- [2.0]
?  ^

+ [1.0]
?  ^
 : different strides


def _triangular_indices_from_tests_arr_k(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

for dtype in [int, float, bool]:
for n in range(10):
for m in range(10):
Copy link
Contributor

Choose a reason for hiding this comment

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

itertools.product!?

Copy link
Member

Choose a reason for hiding this comment

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

yes

arr = np.ones((n, m), dtype)
for k in range(-10, 10):
expected = pyfunc(arr)
got = cfunc(arr)
self.assertEqual(type(expected), type(got))
self.assertEqual(len(expected), len(got))
for e, g in zip(expected, got):
np.testing.assert_array_equal(e, g)

def _triangular_indices_exceptions(self, pyfunc):
cfunc = jit(nopython=True)(pyfunc)

# Exceptions leak references
Copy link
Contributor

Choose a reason for hiding this comment

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

this should be resolvable at typing time?

Copy link
Member

Choose a reason for hiding this comment

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

indeed

self.disable_leak_check()

with self.assertTypingError() as raises:
cfunc(4, k=1.5)
assert "k must be an integer" in str(raises.exception)
Copy link
Member

Choose a reason for hiding this comment

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

If possible, can you add some code to test the exceptions for m and n too?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, please use self.assertIn(), Numba employs unittest style testing.


def _triangular_indices_from_exceptions(self, pyfunc, test_k=True):
cfunc = jit(nopython=True)(pyfunc)

# Exceptions leak references
self.disable_leak_check()

for ndims in [0, 1, 3]:
a = np.ones([5] * ndims)
with self.assertRaises(ValueError) as raises:
cfunc(a)
assert "input array must be 2-d" in str(raises.exception)

if test_k:
a = np.ones([5, 5])
with self.assertTypingError() as raises:
cfunc(a, k=0.5)
assert "k must be an integer" in str(raises.exception)

def test_tril_basic(self):
self._triangular_matrix_tests_m(tril_m)
self._triangular_matrix_tests_m_k(tril_m_k)

def test_tril_exceptions(self):
self._triangular_matrix_exceptions(tril_m_k)

def test_tril_indices(self):
self._triangular_indices_tests_n(tril_indices_n)
self._triangular_indices_tests_n_k(tril_indices_n_k)
self._triangular_indices_tests_n_k_m(tril_indices_n_k_m)
self._triangular_indices_exceptions(tril_indices_n_k)
self._triangular_indices_exceptions(tril_indices_n_k_m)

def test_tril_indices_from(self):
self._triangular_indices_from_tests_arr(tril_indices_from_arr)
self._triangular_indices_from_tests_arr_k(tril_indices_from_arr_k)
self._triangular_indices_from_exceptions(tril_indices_from_arr, False)
self._triangular_indices_from_exceptions(tril_indices_from_arr_k, True)

def test_triu_basic(self):
self._triangular_matrix_tests_m(triu_m)
self._triangular_matrix_tests_m_k(triu_m_k)

def test_triu_exceptions(self):
self._triangular_matrix_exceptions(triu_m_k)

def test_triu_indices(self):
self._triangular_indices_tests_n(triu_indices_n)
self._triangular_indices_tests_n_k(triu_indices_n_k)
self._triangular_indices_tests_n_k_m(triu_indices_n_k_m)
self._triangular_indices_exceptions(triu_indices_n_k)
self._triangular_indices_exceptions(triu_indices_n_k_m)

def test_triu_indices_from(self):
self._triangular_indices_from_tests_arr(triu_indices_from_arr)
self._triangular_indices_from_tests_arr_k(triu_indices_from_arr_k)
self._triangular_indices_from_exceptions(triu_indices_from_arr, False)
self._triangular_indices_from_exceptions(triu_indices_from_arr_k, True)

def partition_sanity_check(self, pyfunc, cfunc, a, kth):
# as NumPy uses a different algorithm, we do not expect to match outputs exactly...
expected = pyfunc(a, kth)
Expand Down