wraparound option not recognized as valid keyword option for jit on CPU target

[spearsem]

I'm trying to use the wraparound keyword argument for jit to disable negative index wrap-around, but this generates a NameError for me. I made a toy example below and show the code pasted into IPython, followed by the traceback. I'm using Anaconda 2.7.10, and Numba 0.22.1.

I did some digging in the code and I can't find a reference to no_wraparound other than the piece of code that handles these keyword arguments.

In particular, you can see for the target I am working with (a CPU), that the OPTIONS dict is set to include the keyword wraparound, whereas the piece of code that attempts to set the flag seems to expect this to be no_wraparound.

I think from what code I read that flags.set is referring to the ConfigOptions.set method defined in utils.py. But this is actually hard to reason about because it's not clear why some of the other options map to alternative names, such as nopython becoming enable_pyobject or looplift becoming enable_looplift when the flags are set ... these names (enable_pyobject and enable_looplift) don't appear to be part of the target OPTIONS dict either, so it's unclear why they do not also raise a NameError. I see where these option names exist in compiler.py, but it's not clear how one goes from the OPTIONS dict of the target to the one defined in the Flags class. Also, wraparound and/or no_wraparound seem to have been missing from that OPTIONS dict in compiler.py at least for several versions going back in the commit history. I didn't bisect to see if I could find precisely where it diverges from the documentation, but it might give you some idea of where to start looking. Some clarification about how exactly that bit of code works would be very helpful. Thanks!

In [77]: %cpaste
Pasting code; enter '--' alone on the line to stop or use Ctrl-D.
:from numba import jit
:
:def py_sum(vals, N):
:    s = 0
:    i = 0
:    while i < N:
:        s += vals[i]
:        i += 1
:    return s
:
:py_sum_jit = jit('int32 (int32[:], int32)', 
:                 nopython=True, 
:                 looplift=True, 
:                 wraparound=False)(py_sum)
:--
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-77-d8e95e59d50e> in <module>()
     12                  nopython=True,
     13                  looplift=True,
---> 14                  wraparound=False)(py_sum)

/home/ely/anaconda/lib/python2.7/site-packages/numba/decorators.pyc in wrapper(func)
    170         if sigs is not None:
    171             for sig in sigs:
--> 172                 disp.compile(sig)
    173             disp.disable_compile()
    174         return disp

/home/ely/anaconda/lib/python2.7/site-packages/numba/dispatcher.pyc in compile(self, sig)
    343 
    344             flags = compiler.Flags()
--> 345             self.targetdescr.options.parse_as_flags(flags, self.targetoptions)
    346 
    347             cres = compiler.compile_extra(self.typingctx, self.targetctx,

/home/ely/anaconda/lib/python2.7/site-packages/numba/targets/options.pyc in parse_as_flags(cls, flags, options)
     25         opt = cls()
     26         opt.from_dict(options)
---> 27         opt.set_flags(flags)
     28         return flags
     29 

/home/ely/anaconda/lib/python2.7/site-packages/numba/targets/options.pyc in set_flags(self, flags)
     45 
     46         if kws.pop('wraparound', True) == False:
---> 47             flags.set("no_wraparound")
     48 
     49         if kws.pop('boundcheck', False):

/home/ely/anaconda/lib/python2.7/site-packages/numba/utils.pyc in set(self, name, value)
    131     def set(self, name, value=True):
    132         if name not in self.OPTIONS:
--> 133             raise NameError("Invalid flag: %s" % name)
    134         self._values[name] = value
    135 

NameError: Invalid flag: no_wraparound

You may also want to elaborate in the documentation if the issue is related to the name of the keyword arg, because in the docstring it makes it seem that wraparound should be OK:

targetoptions: 
    For a cpu target, valid options are:
        nopython: bool
            Set to True to disable the use of PyObjects and Python API
            calls. The default behavior is to allow the use of PyObjects
            and Python API. Default value is False.

        forceobj: bool
            Set to True to force the use of PyObjects for every value.
            Default value is False.

        looplift: bool
            Set to True to enable jitting loops in nopython mode while
            leaving surrounding code in object mode. This allows functions
            to allocate NumPy arrays and use Python objects, while the
            tight loops in the function can still be compiled in nopython
            mode. Any arrays that the tight loop uses should be created
            before the loop is entered. Default value is True.

        wraparound: bool
            Set to True to enable array indexing wraparound for negative
            indices, for a small performance penalty. Default value
            is True.

[pitrou]

We actually removed the wraparound flag at some point, but apparently failed to remove it from the docstring as well...

[spearsem]

@pitrou I also found that wraparound also still appears in the target OPTIONS dict (at least for CPU target). It's not merely in the jit docstring, but also in the dictionary of valid flags. It probably needs to be removed from all these places. As an aside, there seem to be some other new flags which appear to be keyword options, but are not yet described in the docstring for jit, such as nogil and no_rewrites.

What is the new status for negative index wraparound? Does the compiler always add this (with the slight performance overhead)? Or does it no longer perform this at all?

[pitrou]

I also found that wraparound also still appears in the target OPTIONS dict

Thanks! I've pushed a commit to fix this.

What is the new status for negative index wraparound? Does the compiler always add this (with the slight performance overhead)?

Yes, the compiler always adds it. Note that, in some cases, LLVM will be able to optimize away the sign check (for example if the index comes from a range() call).

[spearsem]

Thanks!

[d1manson]

Am I right in thinking that bounds checking is now off (and cannot be turned on) but negative indexing is now on (and cannot be turned off)? That's kind of strange...at the very least this should be made clear in the intro docs (if it isn't already).

Do you have any sense of how performance varies across each of the 2**2=4 combinations of checks?

Ideally it would be nice to be able to control the state of those checks on a per-variable basis, but that's just being greedy.

[pitrou]

Am I right in thinking that bounds checking is now off (and cannot be turned on) but negative indexing is now on (and cannot be turned off)?

Yes, you are right. Negative indexing is a regular Python feature while off-bounds access is a bug which shouldn't happen in correct code; it makes more sense to enable the former even at the possible cost of a small runtime hit.

Do you have any sense of how performance varies across each of the 2**2=4 combinations of checks?

I haven't made any measurement for bounds checking. Negative indexes can be totally costless in many cases where the compiler can infer that an index is always positive (or always negative). In some more complicated cases the test will remain at runtime and add a bit of overhead.

Ideally it would be nice to be able to control the state of those checks on a per-variable basis, but that's just being greedy.

Actually we would rather not provide a ton of low-level knobs to influence code generation; besides being costly maintenance-wise, it also leads users to focus on getting those knobs "right" even though they may not matter at all.
(sort of the register keyword in C, if you know what I mean)

[d1manson]

I take your point about too many low-level knobs.

I could be completely off the mark here, but...

It may well be that even if there are a few extra machine instructions they play to the strengths of modern processors (in terms of branch predicts, cache prefetching, out of order execution etc.) and thus don't actually end up causing any kind of performance hit at all....though intuitively it feels like this would be more true for bounds checking than for wraparound indexing: for example, the compiler might emit predicated gather instructions asking the CPU to get both negative and positive versions of the index, resulting in a polluted cache; alternatively (and this may need to be the case in order to prevent segfaults?) the compiler may emit true branching instructions, which would be zero-overhead if all indices are non-negative, but cause terrible performance if negative indices are randomly interspersed among positive ones...

...what I'm trying to say is that maybe bounds-checking should be standard (if it is indeed near-free), and if wraparound is relatively expensive (which may or may not be the case?) then it would be nice to turn it off. Do you have benchmarks from before you removed the wraparound option?

[pitrou]

what I'm trying to say is that maybe bounds-checking should be standard (if it is indeed near-free)

I am rather skeptical it would be near-free. It should be harder to elide by the compiler than wraparound is (whether an index is always positive is often easy to determine by the compiler, for example if iterating on range(array.shape[0])).

It's true that we should check sometimes what is the slowdown from adding bounds-checking in our benchmarks suite.

[d1manson]

range(something) is an easy case but there are plenty of cases when the compiler certainly can't (or would struggle) to predict: e.g. if one of the inputs to the function is an array of indices, or if the function generates an array of indices from some kind of input data and then subsequently iterates over it several times.

Why wouldn't bounds checking be near-free? The index data is already in register and the branch predictor will be 100% convinced that the index is going to be within bounds (because if it's not we bail out of the whole function). The actual test can hopefully be done in parallel with other instructions.

[pitrou]

e.g. if one of the inputs to the function is an array of indices, or if the function generates an array of indices from some kind of input data and then subsequently iterates over it several times.

This is true (except if you declare the array as unsigned, in which case the compiler knows the indices are positive).

the branch predictor will be 100% convinced that the index is going to be within bounds

The same is true of wraparound: if the index is never negative, the branch predictor will always be right as well.
If the index is sometimes negative then you need wraparound regardless of its cost.

[d1manson]

Ok, I hadn't thought about passing unsigned arrays in this context, that's a good solution.

Regarding the wraparound branch predict, that's the point I was trying to make earlier...but as I said it depends on whether the compiler is emitting predicated instructions or full-on branches.