Implemented `np.allclose` in `numba/np/arraymath.py` by czgdp1807 · Pull Request #7940 · numba/numba

czgdp1807 · 2022-03-26T09:57:00Z

I am implementing np.allclose in numba/np/arraymath.py as I noticed that it hasn't been added yet and I was unable to find any open PR already working on it. Please let me know if its already there or someone recently is already working on it, I will close the PR then.

I am working on adding unit test and therefore I added [skip ci] in my commit so that my PR doesn't consume CI resources unnecessarily. Meanwhile if you have any comments/reviews for the implementation, please let me know. Thanks.

czgdp1807 · 2022-03-26T09:59:10Z

Seems like [skip ci] didn't work.

czgdp1807 · 2022-04-01T11:39:30Z

+        a = np.asarray(a)
+        b = np.asarray(b)
+        if a.shape != b.shape:
+            if _can_broadcast_returns_bool(a, b.shape):
+                a = np.broadcast_to(a, b.shape)
+            elif _can_broadcast_returns_bool(b, a.shape):
+                b = np.broadcast_to(b, a.shape)
+            else:
+                raise ValueError('operands could not be broadcast together '
+                                 'with remapped shapes')


The broadcasting here fails if one of a or b is a scalar. The reason being, .shape will be ()(an empty UniTuple) and it will fail while JITing the code.

As you notice, one cannot call np.broadcast_to with an empty shape. In the case one of the arguments is an integer, you can iterate over the array and check if they're the same:

if isinstance(b, types.Integer): def impl(a, b, rtol=1e-05, atol=1e-08, equal_nan=False): for av in np.nditer(a): if not allclose_inner(av, b, rtol, atol, equal_nan): return False return True return impl else: def np_allclose_impl(a, b, rtol=1e-05, atol=1e-08, equal_nan=False): ... return np_allclose_impl

You can replicate the idea above for the cases where

both a and b are scalars

either a or b is scalar

both a and b are arrays

This sounds like a nice approach. Thanks for the idea.

czgdp1807 · 2022-04-01T16:35:24Z

@guilhermeleobas Could you please review this PR and provide your thoughts/opinions on the above comments? Thanks.

guilhermeleobas

Hi @czgdp1807, thanks for the contribution to the Numba codebase. This is a nice addition. I've made a few comments regarding the usage of assert and answered your questions on how you can handle mixed input types (i.e. array and int).

Also, you can copy the inputs NumPy uses to test np.allclose to make sure we're covering all cases.
https://github.com/numpy/numpy/blob/aac965af6032b69d5cb515ad785cc9a331e816f4/numpy/core/tests/test_numeric.py#L2203-L2285

guilhermeleobas · 2022-04-01T20:38:23Z

+
+        a = np.asarray([1e10, 1e-7])
+        b = np.asarray([1.00001e10, 1e-8])
+        assert not cfunc(a, b)


Replace the assert by self.assertEqual.

I though that since, output of cfunc and pyfunc is a bool object so using assert should be fine. Though I am fine with self.assertEqual as well. Will do the replacements.

guilhermeleobas · 2022-04-01T20:43:52Z

+        a = np.asarray([1e10, 1e-7])
+        b = np.asarray([1.00001e10, 1e-8])


You can aggregate the input values into a list (or tuple) and do the assertion inside the loop body:

pyfunc = np_allclose cfunc = jit(nopython=True)(pyfunc) inps = [ (np.asarray([1e10, 1e-7]), np.asarray([1.00001e10, 1e-8]), (np.asarray([1e10, 1e-8]), np.asarray([1.00001e10, 1e-9]), (np.asarray([1e10, 1e-8]), np.asarray([1.0001e10, 1e-9])), (np.asarray([1e10]), np.asarray([1.0001e10, 1e-9])), # .... ] for a, b in inps: expected = pyfunc(a, b) got = cfunc(a, b) self.assertEqual(expected, got)

guilhermeleobas · 2022-04-02T04:35:28Z

+        a = np.asarray(a)
+        b = np.asarray(b)
+        if a.shape != b.shape:
+            if _can_broadcast_returns_bool(a, b.shape):
+                a = np.broadcast_to(a, b.shape)
+            elif _can_broadcast_returns_bool(b, a.shape):
+                b = np.broadcast_to(b, a.shape)
+            else:
+                raise ValueError('operands could not be broadcast together '
+                                 'with remapped shapes')


As you notice, one cannot call np.broadcast_to with an empty shape. In the case one of the arguments is an integer, you can iterate over the array and check if they're the same:

if isinstance(b, types.Integer): def impl(a, b, rtol=1e-05, atol=1e-08, equal_nan=False): for av in np.nditer(a): if not allclose_inner(av, b, rtol, atol, equal_nan): return False return True return impl else: def np_allclose_impl(a, b, rtol=1e-05, atol=1e-08, equal_nan=False): ... return np_allclose_impl

You can replicate the idea above for the cases where

both a and b are scalars

either a or b is scalar

both a and b are arrays

guilhermeleobas · 2022-04-02T04:39:42Z

+        with self.assertRaises(ValueError):
+            cfunc(a, b)


Move the test that raises an exception to its own function. If I'm not wrong, one need to disable the leak check when asserting exceptions:

def test_allcase_exception(self): self.disable_leak_check() a = np.asarray([1e10, 1e-9, np.nan]) b = np.asarray([1.0001e10, 1e-9]) with self.assertRaises(ValueError) as e: cfunc(a, b) self.assertIn("put here the expected exception msg", str(e.exception))

guilhermeleobas · 2022-04-02T04:40:42Z

+
+                    py_result = pyfunc(a + noise, a, atol=atol, rtol=rtol)
+                    c_result = cfunc(a + noise, a, atol=atol, rtol=rtol)
+                    assert py_result == c_result


guilhermeleobas · 2022-04-02T04:40:59Z

+        assert not cfunc(a, b)
+        assert cfunc(a, b, equal_nan=True)


Replace the assert by self.assertEqual

guilhermeleobas · 2022-04-02T04:41:17Z

+
+        a = np.asarray([1e10, 1e-8])
+        b = np.asarray([1.00001e10, 1e-9])
+        assert cfunc(a, b)


Replace the assert by self.assertEqual.

guilhermeleobas · 2022-04-02T04:41:21Z

+
+        a = np.asarray([1e10, 1e-8])
+        b = np.asarray([1.0001e10, 1e-9])
+        assert not cfunc(a, b)


Replace the assert by self.assertEqual.

guilhermeleobas · 2022-04-02T04:41:26Z

+
+        a = np.asarray([1e10])
+        b = np.asarray([1.0001e10, 1e-9])
+        assert not cfunc(a, b)


Replace the assert by self.assertEqual.

czgdp1807 · 2022-04-02T09:41:30Z

@guilhermeleobas I have addressed the reviews and responded to the questions.

guilhermeleobas

Good work @czgdp1807. I have a few more comments that should be easy to address.

guilhermeleobas · 2022-04-06T19:31:31Z

+    elif is_a_scalar and not is_b_scalar:
+        def np_allclose_impl_scalar_array(a, b, rtol=1e-05, atol=1e-08,
+                                          equal_nan=False):
+            b = np.asarray(b)
+            for bv in np.nditer(b):
+                if not _allclose_scalars(a, bv.item(), rtol=rtol, atol=atol,
+                                         equal_nan=equal_nan):
+                    return False
+            return True
+        return np_allclose_impl_scalar_array


You can replace this block of code by:

Suggested change

elif is_a_scalar and not is_b_scalar:

def np_allclose_impl_scalar_array(a, b, rtol=1e-05, atol=1e-08,

equal_nan=False):

b = np.asarray(b)

for bv in np.nditer(b):

if not _allclose_scalars(a, bv.item(), rtol=rtol, atol=atol,

equal_nan=equal_nan):

return False

return True

return np_allclose_impl_scalar_array

elif is_a_scalar and not is_b_scalar:

def np_allclose_impl_scalar_array(a, b, rtol=1e-05, atol=1e-08,

equal_nan=False):

return np.allclose(b, a, rtol=rtol, atol=atol, equal_nan=equal_nan)

return np_allclose_impl_scalar_array

I don't think this will give right results all the time, because the check depends on the ordering of a and b. For (a, b) the check is abs(a - b) > atol + rtol * abs(b), where as for (b, a) the check is abs(b - a) > atol + rtol * abs(a). See the RHS in both the inequalities, one uses b and the other uses a. So, np.allclose(a, b) might give different result from np.allclose(b, a) for edge cases.

Quoting from "Notes" section of https://numpy.org/doc/stable/reference/generated/numpy.allclose.html

The above equation is not symmetric in a and b, so that allclose(a, b) might be different from allclose(b, a) in some rare cases.

Please let me know if should still apply the above code suggestion.

That's fine. I wasn't aware of this info.

guilhermeleobas · 2022-04-06T19:33:25Z

+        b = np.asarray([np.nan, 1.0])
+        self.assertFalse(cfunc(a, b))
+
+        # NumPy test data


Can you also include the link to NumPy repository pointing to the tests?

guilhermeleobas · 2022-04-06T19:34:12Z

+        b = np.asarray([np.nan, 1.0])
+        self.assertFalse(cfunc(a, b))
+
+        # NumPy test data


Maybe split the tests into different functions. This makes it easy to debug when there's a test failure.

guilhermeleobas · 2022-04-06T19:37:26Z

+                    c_result = cfunc(a + noise, a, atol=atol, rtol=rtol)
+                    self.assertEqual(py_result, c_result)
+
+    def test_allcase_exception(self):


Suggested change

def test_allcase_exception(self):

def test_allclose_exception(self):

czgdp1807 · 2022-04-07T13:33:02Z

@guilhermeleobas @gmarkall I have addressed/responded to the new reviews and the tests pass. Please let me know if something else is to be done.

guilhermeleobas

Thanks, @czgdp1807, I only have a few more requests.

guilhermeleobas · 2022-04-07T23:22:35Z

+    def test_allclose_numpy_data(self):
+
+        pyfunc = np_allclose


Can you split the content of this test case into different functions? Similar to what NumPy does.

guilhermeleobas · 2022-04-07T23:23:37Z

+        ]
+
+        for (x, y) in numpy_data:
+            self.assertTrue(cfunc(x, y))


Instead of asserting to True, compare the outputs of cfunc(x, y) and pyfunc(x, y) are equals.

guilhermeleobas · 2022-04-07T23:23:56Z

+        ]
+
+        for (x, y) in numpy_data:
+            self.assertFalse(cfunc(x, y))


Same here. Compare the outputs of cfunc(x, y) and pyfunc(x, y).

guilhermeleobas · 2022-04-07T23:25:03Z

+        raise TypeError('The first argument "b" must be array-like')
+
+    if not isinstance(rtol, types.Float):
+        raise TypeError('The fourth argument "rtol" must be a '


rtol is the third argument, right?

Suggested change

raise TypeError('The fourth argument "rtol" must be a '

raise TypeError('The third argument "rtol" must be a '

guilhermeleobas · 2022-04-07T23:25:14Z

+                        'floating point')
+
+    if not isinstance(atol, types.Float):
+        raise TypingError('The fifth argument "atol" must be a '


Suggested change

raise TypingError('The fifth argument "atol" must be a '

raise TypingError('The fourth argument "atol" must be a '

guilhermeleobas · 2022-04-07T23:25:23Z

+                          'floating point')
+
+    if not isinstance(equal_nan, types.Boolean):
+        raise TypeError('The sixth argument "equal_nan" must be a '


Suggested change

raise TypeError('The sixth argument "equal_nan" must be a '

raise TypeError('The fifth argument "equal_nan" must be a '

guilhermeleobas · 2022-04-07T23:30:20Z

+        self.assertFalse(cfunc(a, b))
+        self.assertTrue(cfunc(a, b, equal_nan=True))


Compare the output of cfunc with pyfunc instead.

guilhermeleobas · 2022-04-07T23:31:52Z

+        self.assertTrue(cfunc(a, b, equal_nan=True))
+
+        b = np.asarray([np.nan, 1.0])
+        self.assertFalse(cfunc(a, b))


Compare the output of cfunc with pyfunc instead.

guilhermeleobas · 2022-04-08T15:40:37Z

@czgdp1807, I just remembered that you need to update the file numpysupported.rst to include np.allclose.

czgdp1807 · 2022-04-08T15:42:52Z

Sure. I will do that tomorrow.

czgdp1807 · 2022-04-09T11:17:45Z

I just remembered that you need to update the file numpysupported.rst to include np.allclose.

Done.

guilhermeleobas

LGTM. Thanks, @czgdp1807

czgdp1807 · 2022-04-14T05:51:41Z

@sklam @stuartarchibald @esc Same for this PR. Is it ready for merge? Please let me know if anything is left to be done here. Thanks.

sklam

This should be the last needed change

sklam · 2022-04-14T23:28:22Z

+        raise TypeError('The first argument "a" must be array-like')
+
+    if not type_can_asarray(b):
+        raise TypeError('The first argument "b" must be array-like')


Suggested change

raise TypeError('The first argument "b" must be array-like')

raise TypeError('The second argument "b" must be array-like')

I have addressed the review and the tests are passing. Please let me know if anything else is to be done here. Thanks.

sklam

Thanks for the patch!

czgdp1807 added 2 commits March 26, 2022 16:44

Added np_allclose in numba/np/arraymath.py

e209bc1

Added test for np_allclose

d16cd63

czgdp1807 commented Mar 26, 2022

View reviewed changes

Comment thread numba/np/arraymath.py Outdated

czgdp1807 commented Mar 26, 2022

View reviewed changes

Comment thread numba/tests/test_np_functions.py Outdated

sklam added the 2 - In Progress label Mar 28, 2022

Added broadcasting support and stronger tests

eee11d0

czgdp1807 commented Apr 1, 2022

View reviewed changes

czgdp1807 marked this pull request as ready for review April 1, 2022 11:41

czgdp1807 requested review from esc, sklam and stuartarchibald as code owners April 1, 2022 11:41

guilhermeleobas requested changes Apr 2, 2022

View reviewed changes

czgdp1807 added 3 commits April 2, 2022 12:46

Handeled scalars and improved tests

2dc5bb7

Added test data from np.allclose tests

ca98b63

Applied linting

6542301

czgdp1807 requested a review from guilhermeleobas April 2, 2022 09:41

czgdp1807 added 4 commits April 2, 2022 15:47

Call a separate function to ensure same rank of arrays

f09d8ff

Applied linting

50367ef

Merge branch 'main' into np_allclose

0cc9234

Use np.broadcast_arrays and remove utility functions

9ee9842

esc added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 2 - In Progress labels Apr 4, 2022

stuartarchibald added the Effort - medium Medium size effort needed label Apr 5, 2022

guilhermeleobas requested changes Apr 6, 2022

View reviewed changes

gmarkall added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Apr 6, 2022

Separate numpy tests for allclose

4d90833

gmarkall added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Apr 7, 2022

guilhermeleobas requested changes Apr 7, 2022

View reviewed changes

czgdp1807 added 2 commits April 8, 2022 12:12

Improved tests of np_allclose

ead6817

Corrected error messages in np_allclose

9c1d44f

Added numpy.allclose to numpysupported.rst

1b24788

guilhermeleobas previously approved these changes Apr 11, 2022

View reviewed changes

sklam reviewed Apr 14, 2022

View reviewed changes

sklam added 4 - Waiting on author Waiting for author to respond to review and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Apr 14, 2022

czgdp1807 added 2 commits April 20, 2022 12:40

Merge branch 'main' into np_allclose

a93b0ef

Corrected argument number in error message

66969ce

czgdp1807 dismissed guilhermeleobas’s stale review via 66969ce April 20, 2022 07:21

sklam approved these changes Apr 20, 2022

View reviewed changes

sklam added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on author Waiting for author to respond to review labels Apr 20, 2022

sklam merged commit 63bb6c3 into numba:main Apr 21, 2022

stuartarchibald added this to the Numba 0.56 RC milestone Apr 25, 2022

swap357 mentioned this pull request Jan 21, 2026

Add np.allclose and np.isclose support ref. issue #4074 #6286

Closed

lispandfound mentioned this pull request Feb 24, 2026

Consistent coordinates for pre-compute and 1d profile generation ucgmsim/velocity_modelling#50

Merged

		a = np.asarray([1e10, 1e-7])
		b = np.asarray([1.00001e10, 1e-8])

	def test_allcase_exception(self):
	def test_allclose_exception(self):

	raise TypeError('The fourth argument "rtol" must be a '
	raise TypeError('The third argument "rtol" must be a '

	raise TypingError('The fifth argument "atol" must be a '
	raise TypingError('The fourth argument "atol" must be a '

	raise TypeError('The sixth argument "equal_nan" must be a '
	raise TypeError('The fifth argument "equal_nan" must be a '

		self.assertFalse(cfunc(a, b))
		self.assertTrue(cfunc(a, b, equal_nan=True))

	raise TypeError('The first argument "b" must be array-like')
	raise TypeError('The second argument "b" must be array-like')

Conversation

czgdp1807 commented Mar 26, 2022

Uh oh!

czgdp1807 commented Mar 26, 2022

Uh oh!

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!

czgdp1807 commented Apr 1, 2022

Uh oh!

guilhermeleobas 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!

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!

czgdp1807 commented Apr 2, 2022

Uh oh!

guilhermeleobas 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!

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!

czgdp1807 commented Apr 7, 2022

Uh oh!

guilhermeleobas 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!

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!

guilhermeleobas commented Apr 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

guilhermeleobas commented Apr 8, 2022 •

edited

Loading