Modules can't be parfor races.

[DrTodd13]

Resolves #5098

In #5098, there are two parfors. The first one defines some variable of type module and that variable is used by the second parfor. Since module vars can't be passed to gufuncs, ParallelAccelerator copies such module vars into parfor loop bodies as needed. However, in this case, since the variable was defined in the first parfor and to liveness analysis was escaping that parfor, it looked like a race condition variable to parfor analysis. Since we do this module var copying later in the pipeline, I just made it such that parfors don't identify module var as races. The reason this caused a problem is that if a variable is a race variable, we create an array for it so that some value can be returned but we can't create arrays of modules and pass them to a gufunc so some of the machinery in the create gufunc code generated an exception which then propagated.

@stuartarchibald Please handle defining what the behavior should be when lowering generates an exception. We probably don't want it to just stop executing if there is a fallback path that could work but we don't want it happening silently either. I'm not sure how to test this particular issue in the test suite either so I'll also leave that up to you. Commits to my PR branch are solicited.

[stuartarchibald]

I've added a patch here stuartarchibald@4b73d72 that makes it so that LoweringErrors that appear during typing cause a hard stop. I think this makes it so that if a LoweringError occurs in the _create_gufunc_for_parfor_body the stack will just unwind and the code will halt:

numba/numba/npyufunc/parfor.py

Lines 235 to 239 in cacb5c4

	numba.parfor.sequential_parfor_lowering = True
	func, func_args, func_sig, redargstartdim, func_arg_types = _create_gufunc_for_parfor_body(
	lowerer, parfor, typemap, typingctx, targetctx, flags, {},
	bool(alias_map), index_var_typ, parfor.races)
	numba.parfor.sequential_parfor_lowering = False

I am of the view that lowering should succeed and it's an error if it does not. In the case presented I don't know what the best thing for parfors to do is, it seems like catching a LoweringError from that function, resetting the flag and then reraising that error might be the sensible thing to do? I'm very reluctant to mess about too much with the exception based control flow in typing as it is an incredibly nuanced area of the code base.

[Hardcode84]

@stuartarchibald

seems like catching a LoweringError from that function, resetting the flag and then reraising that error might be the sensible thing to do?

There is already patch for that #5114

Regarding your patch - this issue was causing AssertError during ir construction initially, not LoweringError iirc. And in general, maybe better instead of whitelisting specific exceptions which we allow to propagate (and AssertError definitely should be in this list regardless of this issue) to list all exceptions we allow to be consumed and propagate all others?

[stuartarchibald]

@stuartarchibald

seems like catching a LoweringError from that function, resetting the flag and then reraising that error might be the sensible thing to do?

There is already patch for that #5114

Ah yes, thanks, forgot about that.

Regarding your patch - this issue was causing AssertError during ir construction initially, not LoweringError iirc. And in general, maybe better instead of whitelisting specific exceptions which we allow to propagate (and AssertError definitely should be in this list regardless of this issue) to list all exceptions we allow to be consumed and propagate all others?

There's a lot of exception class rewriting and chaining going on, typically exceptions are rewritten to TypingError if they occurred during typing or LoweringError if they occurred during lowering. I think that this makes it relatively safe to say "if your code raises a LoweringError during typing, then propagate".

[Hardcode84]

if your code raises a LoweringError during typing, then propagate

In this case opposite should be also true, e.g. consume only TypingError during typing and only LoweringError during lowering

[stuartarchibald]

As discussed out of band, I think that this can be merged once the conflicts are resolved?

[sklam]

Merged via #5247