Add support for arrayanalysis of tuple args. by DrTodd13 · Pull Request #7398 · numba/numba

DrTodd13 · 2021-09-13T23:40:56Z

Previously, if a tuple of integers were passed in and then used in a pndindex, the parfor pass would fail because the tuple of dimension sizes didn't have an entry in array analysis. This PR adds integer tuple arguments to array analysis whereas previously only integer tuple variables were supported.

gmarkall · 2021-09-14T09:19:43Z

@DrTodd13 Thanks for the PR! Is there a related issue for this PR?

stuartarchibald · 2021-09-14T13:53:22Z

Thanks for the patch. I wonder if adding the expression to equivalence analysis creates a problem for legitimate cases such as this:

from numba import njit, pndindex
import numpy as np

@njit(parallel=True)
def test_impl(x, sz):
    for i in pndindex(sz):
        x[i] = 1
    return x

@njit(parallel=True)
def call(x):
    return test_impl(x, (10, 3)) # Only want to iterate the 3rd column

print(call(np.empty((10, 10))))

where a user wants to just iterate to the 3rd column of the 2d array.

DrTodd13 · 2021-09-14T15:16:08Z

@DrTodd13 Thanks for the PR! Is there a related issue for this PR?

No. It was just something that I ran across in the course of my regular work that I needed to work in Numba.

DrTodd13 · 2021-09-14T15:26:30Z

@stuartarchibald If I don't make "call" njit then your example works. If I make "call" njit then I get an internal typing error at numba.core.typeinfer.CallConstraint which doesn't look related to my changes.

stuartarchibald · 2021-09-15T10:20:24Z

@stuartarchibald If I don't make "call" njit then your example works. If I make "call" njit then I get an internal typing error at numba.core.typeinfer.CallConstraint which doesn't look related to my changes.

I see the same, it's a chained exception, it looks a bit like:

<snip>
numba/parfors/parfor.py", line 2441, in run
    assert(equiv_set.has_shape(args[0]))
AssertionError: Failed in nopython mode pipeline (step: convert to parfors)


The above exception was the direct cause of the following exception:

<snip>
numba/core/typeinfer.py", line 1074, in propagate
    raise errors[0]
numba.core.errors.TypingError: Failed in nopython mode pipeline (step: nopython frontend)
Internal error at <numba.core.typeinfer.CallConstraint object at 0x7fe012437940>.
Failed in nopython mode pipeline (step: convert to parfors

i.e. an AssertionError in parfors triggered it.

…l integer arguments.

DrTodd13 · 2021-09-17T18:12:57Z

@stuartarchibald When you make the caller Numba then the tuple (10,3) doesn't become a UniTuple of Literal[int] but a regular Tuple of two Literal[int]. Is there a reason for that? In any case, maybe that is better because you can also have a Tuple with one literal and the other a non-literal which my recent patch to this PR supports. Please have another look. I added an additional test for this mixed case (which subsumes the case of two literals).

…sional.

stuartarchibald · 2021-10-05T17:00:17Z

@stuartarchibald When you make the caller Numba then the tuple (10,3) doesn't become a UniTuple of Literal[int] but a regular Tuple of two Literal[int]. Is there a reason for that?

I think it's because the IR preserves literals into the tuple type during type inference.

numba/numba/core/typeinfer.py

Lines 230 to 241 in 4a3d575

    
           def __call__(self, typeinfer): 
        
               with new_error_context("typing of tuple at {0}", self.loc): 
        
                   typevars = typeinfer.typevars 
        
                   tsets = [typevars[i.name].get() for i in self.items] 
        
                   for vals in itertools.product(*tsets): 
        
                       if vals and all(vals[0] == v for v in vals): 
        
                           tup = types.UniTuple(dtype=vals[0], count=len(vals)) 
        
                       else: 
        
                           # empty tuples fall here as well 
        
                           tup = types.Tuple(vals) 
        
                       assert tup.is_precise() 
        
                       typeinfer.add_type(self.target, tup, loc=self.loc)

This bit:

if vals and all(vals[0] == v for v in vals):

will be False as the literal values will compare False. I think this might be from trying to preserve as much constant type information as possible.

In any case, maybe that is better because you can also have a Tuple with one literal and the other a non-literal which my recent patch to this PR supports. Please have another look. I added an additional test for this mixed case (which subsumes the case of two literals).

I think that this is indeed the case.

stuartarchibald

Thanks for the update. I've taken a look through and have left some new comments inline. I also wonder if it'd be a good idea to include the test case from here: #7398 (comment) somewhere so as to ensure this common pattern works.

Thanks again!

stuartarchibald · 2021-10-05T17:09:22Z

+            for i in numba.pndindex(sz):
+                x[i] = 1
+            return x
+        sz = (10,numba.literally(10))


I'm not sure that this has the desired effect. numba.literally outside of compiled code is a pass through NOP:

numba/numba/misc/special.py

Lines 64 to 87 in 4a3d575

def literally(obj):

"""Forces Numba to interpret *obj* as an Literal value.

*obj* must be either a literal or an argument of the caller function, where

the argument must be bound to a literal. The literal requirement

propagates up the call stack.

This function is intercepted by the compiler to alter the compilation

behavior to wrap the corresponding function parameters as ``Literal``.

It has **no effect** outside of nopython-mode (interpreter, and objectmode).

The current implementation detects literal arguments in two ways:

1. Scans for uses of ``literally`` via a compiler pass.

2. ``literally`` is overloaded to raise ``numba.errors.ForceLiteralArg``

to signal the dispatcher to treat the corresponding parameter

differently. This mode is to support indirect use (via a function call).

The execution semantic of this function is equivalent to an identity

function.

See :ghfile:`numba/tests/test_literal_dispatch.py` for examples.

"""

return obj

it might be necessary to close over or locally define a const to get a literal if that's the intent?

I've added some tests and tried to generate the right tuples using what I think you were suggesting above with one literal arg but there is some lowering error I don't understand. Can you take a look at it?

…er arg tuple with mix of literal and non-literal integer. Add additional tuple tests in combination with pndindex.

… creates one loop nest with an integer index variable. In other words, it becomes much like a prange in that case.

stuartarchibald · 2021-10-06T20:37:06Z

+                        in_arr_typ = pass_states.typemap[in_arr.name]
+                        if isinstance(in_arr_typ, types.Integer):
+                            index_var = ir.Var(scope, mk_unique_var("parfor_index"), loc)
+                            pass_states.typemap[index_var.name] = types.uintp


Not to be fixed in this PR. But I noticed that both njit and njit(parallel=True) will compiled and execute a pndindex driven loop where the arg supplied to pndindex is a negative integer. e.g.

@njit(parallel=True) def test_impl(x, sz): for i in pndindex(sz): x[i] = 1 return x print(test_impl(np.zeros((10, 10)), -3))

Tracked in #7462

stuartarchibald

Thanks for the patch and fixes.

Add support for arrayanalysis of tuple args.

36a9da7

DrTodd13 requested review from esc, sklam and stuartarchibald as code owners September 13, 2021 23:40

gmarkall added 3 - Ready for Review ParallelAccelerator labels Sep 14, 2021

stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Sep 14, 2021

DrTodd13 added 4 commits September 16, 2021 18:21

Support tuples of integer literals in alias analysis.

85e5548

Support tuples that are combinations of integer vars and literals.

c3406ce

Add test for pndindex of tuple arg with mix of literal and non-litera…

435d434

…l integer arguments.

Fix flake8 problems.

6be6198

DrTodd13 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 Sep 17, 2021

DrTodd13 added 2 - In Progress and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Sep 17, 2021

DrTodd13 added 2 commits September 17, 2021 15:48

Force index vars for pndindex to be constructed as tuples for 1 dimen…

96981f2

…sional.

Add tests for 1d pndindex.

1f922eb

DrTodd13 added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 2 - In Progress labels Sep 20, 2021

stuartarchibald reviewed Oct 5, 2021

View reviewed changes

stuartarchibald 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 Oct 5, 2021

stuartarchibald added this to the Numba 0.55 RC milestone Oct 5, 2021

stuartarchibald added the Effort - medium Medium size effort needed label Oct 5, 2021

DrTodd13 added 6 commits October 5, 2021 13:30

Add comment for force_tuple param of make_index_var.

98a6923

Use numba.literally inside numba function to pass to another to trigg…

68047d3

…er arg tuple with mix of literal and non-literal integer. Add additional tuple tests in combination with pndindex.

Add test and fix mistake in new parfor tests.

cdd7a26

fix scope issue

8c06789

Remove unneeded literally.

29c1440

more arg tuple literal fixes

ed34eb5

stuartarchibald reviewed Oct 6, 2021

View reviewed changes

Comment thread numba/tests/test_parfors.py

stuartarchibald reviewed Oct 6, 2021

View reviewed changes

Comment thread numba/tests/test_parfors.py Outdated

DrTodd13 added 3 commits October 6, 2021 09:39

Add manual test of mixed tuple of int and literal.

99dec34

Add integer argument test for pndindex on 2d array.

391893a

Add ability for pndindex to take an integer argument in which case it…

e39f6c9

… creates one loop nest with an integer index variable. In other words, it becomes much like a prange in that case.

stuartarchibald reviewed Oct 6, 2021

View reviewed changes

stuartarchibald approved these changes Oct 6, 2021

View reviewed changes

stuartarchibald 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 Oct 6, 2021

stuartarchibald mentioned this pull request Oct 6, 2021

numba.pndindex accepts negative integers #7462

Open

2 tasks done

sklam merged commit f780d59 into numba:master Oct 6, 2021

	def literally(obj):
	"""Forces Numba to interpret obj as an Literal value.

	obj must be either a literal or an argument of the caller function, where
	the argument must be bound to a literal. The literal requirement
	propagates up the call stack.

	This function is intercepted by the compiler to alter the compilation
	behavior to wrap the corresponding function parameters as ``Literal``.
	It has no effect outside of nopython-mode (interpreter, and objectmode).

	The current implementation detects literal arguments in two ways:

	1. Scans for uses of ``literally`` via a compiler pass.
	2. ``literally`` is overloaded to raise ``numba.errors.ForceLiteralArg``
	to signal the dispatcher to treat the corresponding parameter
	differently. This mode is to support indirect use (via a function call).

	The execution semantic of this function is equivalent to an identity
	function.

	See :ghfile:`numba/tests/test_literal_dispatch.py` for examples.
	"""
	return obj

Conversation

DrTodd13 commented Sep 13, 2021

Uh oh!

gmarkall commented Sep 14, 2021

Uh oh!

stuartarchibald commented Sep 14, 2021

Uh oh!

DrTodd13 commented Sep 14, 2021

Uh oh!

DrTodd13 commented Sep 14, 2021

Uh oh!

stuartarchibald commented Sep 15, 2021

Uh oh!

DrTodd13 commented Sep 17, 2021

Uh oh!

stuartarchibald commented Oct 5, 2021

Uh oh!

stuartarchibald left a comment

Choose a reason for hiding this comment

Uh oh!

stuartarchibald Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

DrTodd13 Oct 5, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

stuartarchibald Oct 6, 2021

Choose a reason for hiding this comment

Uh oh!

stuartarchibald Oct 6, 2021

Choose a reason for hiding this comment

Uh oh!

stuartarchibald left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants