ENH: cython_special: GIL free error handling

[person142]

This changes the error handling in cython_special to be based on a global errno-style variable. With these changes error handling looks like:

import scipy.special.cython_special as sc

cdef double res
cdef sc.error_t err

res = sc.spence(-1.0)
err = sc.get_errno()
if err != sc.OK:
    # Do something

To achieve that effect with the old error handling system one would have to do something like:

cimport scipy.special.cython_special as sc
import warnings

cdef double res

warnings.simplefilter('raise')
try:
    res = sc.spence(-1.0)
except:
    # Do something

Note that with the old error handling changing the flow of the program based on errors requires grabbing the GIL.

If SciPy is compiled by a C compiler with OpenMP support, then the new error handling is threadsafe for OpenMP threads. In particular this covers uses of cython.parallel; e.g. something like the following code:

from cython cimport parallel
cimport scipy.special.cython_special as sc

cdef int n
for n in parallel.prange(10):
    sc.loggamma(<double>n)
    if sc.get_errno() != sc.OK:
        # Do something

is fine.

See #6681 for more discussion.

Some aspects of this PR are ugly and could perhaps be done in a nicer way; in particular it currently implements a custom build_ext in the top-level setup.py that tries to detect whether the C compiler supports OpenMP. A better way of doing this would be welcome. (Presumably changing numpy.distutils would be a better way of doing this, but ISTM it is expedient to get this figured out before cython_special is in a release.)

[person142]

I'm running into a bit of a problem, and I wonder if anyone has any ideas.

How things work now:

• The C and C++ functions are in separate extension modules; the C++ functions are exported to the C module as pointers.
• Error printing is controlled by a global variable print_error_messages; each extension module has its own copy of this variable.
• The function special.errprint lives in the C module and controls both copies of print_error_messages (using accessor functions).

To make the necessary modifications to cython_special error handling we need to instead share a single global variable between the two extension modules. On gcc we could do this with -rpath, but coming up with a portable solution seems messy. Is there a nice way out of this?

[rgommers]

in particular it currently implements a custom build_ext in the top-level setup.py that tries to detect whether the C compiler supports OpenMP

I have a bad feeling about this ....:

• it's hard to predict if this build_ext class is going to have a bad interaction with something else in distutils/setuptools/pip/...
• the list of compiler names is incomplete, so using the new if err != sc.OK: in user case may give surprising results on less common platforms.
• IIRC we have never allowed OpenMP in SciPy because it's not portable enough. Can't remember the details though, could be that the limited use here is okay.

[person142]

I have a bad feeling about this ....

Yeah, pretty much me too. I don't like the other options either, which seem to be:

• Leave things as they are. Functions can emit warnings. I realized that I was wrong above; doing somethings like warnings.simplefilter('error') will actually cause all exceptions to be ignored! This is probably my fault, maybe in sf_error.c changing the warnings filter makes PyErr_WarnEx always return an error status which I then accidentally clear with PyErr_Clear? Even if that's fixed it probably still won't work properly due to not having a way to propagate the errors.
• Change every function to have a *error argument which we use to return errors. (Requires a lot of changes, maybe doable? In theory I like this but the practicalities are probably nasty.)
• Don't report any errors

Not sure where to go; the current situation is undesirable enough that something should be done though.

[person142]

I think I'm inclined to say "let's turn off error reporting". The current method is buggy, and other methods have their own problems. Better to have nothing and add something if the situation improves than have something bad. Opinions?

[pv]

You could have the TLS flag owned by the C++ module, and export functions from there for toggling its value.

I don't really like using openmp for this, and the fact that the flag becomes non-threadlocal if you don't have openmp on.

We can always add cython_special.get_errno() etc. later on, if it becomes more clear what is the best way to do it. I don't think there's a principal obstacle for adding it later --- the special function errors are also often not that useful, as you may not care specifically for the precise reason why some function returned nans.

[person142]

Ok, let's close this then and I'll submit something which removes the current (buggy) error handling. (The relief I feel at not having to mess with distutils anymore is palpable!)

[rgommers]

The relief I feel at not having to mess with distutils anymore is palpable!

a familiar feeling:)