gh-155526: correct errno handling in complex_abs()

[skirpichev]

• Issue: complex_abs() should call _Py_c_abs() with errno=0 set #155526

[skirpichev]

CC @vstinner

[skirpichev]

There is a faster alternative to this, using hypot directly.

The C standard says:

Each of the functions cabs and carg is specified by a formula in terms of a real function (whose special cases are covered in Annex F):
cabs(x + iy ) = hypot(x, y )
carg(x + iy ) = atan2(y , x)

I got (default ./configure flags, gcc 14.2):

Mean +- std dev: [ref] 259 ns +- 3 ns -> [patch2] 231 ns +- 10 ns: 1.12x faster

with

diff --git a/Objects/complexobject.c b/Objects/complexobject.c
index 3612c2699a5..1554fe4905a 100644
--- a/Objects/complexobject.c
+++ b/Objects/complexobject.c
@@ -796,8 +796,13 @@ static PyObject *
 complex_abs(PyObject *op)
 {
     PyComplexObject *v = _PyComplexObject_CAST(op);
-    double result = _Py_c_abs(v->cval);
-    if (errno == ERANGE) {
+    double result;
+
+    result = hypot(v->cval.real, v->cval.imag);
+    /* Testing FE_OVERFLOW floating-point exception is slow. */
+    if (isfinite(v->cval.real) && isfinite(v->cval.imag)
+        && !isfinite(result))
+    {
         PyErr_SetString(PyExc_OverflowError,
                         "absolute value too large");
         return NULL;

Let me know if you prefer this version. See also #156145.

Details

# bench.py
import pyperf

z = complex(3.140625, 1.0)

runner = pyperf.Runner()
runner.bench_func("abs(z)", abs, z)

[hpkfft]

The function _Py_c_abs() does special value testing itself (if either real or imag part is inf, return inf even if the other part is NaN), so it does not rely on the platform's math library to do this correctly (as required by Annex F). Can we rely on the platform math library to do this correctly? If so, then the special value testing can be removed from _Py_c_abs().

I wonder how much performance gain came from not checking errno (which hopefully we'll get from merging the enhancement "issue" you noted above) and how much came from removing/skipping the special value testing and relying on the C math library to do it. (Well, I saw 1.07x for the former, but on very different hardware.)

[skirpichev]

If so, then the special value testing can be removed from _Py_c_abs().

This is soft-deprecated API function. Technically, we could also drop everything here, except for hypot() call and testing it's output value. At price that will break some exotic platform.

As there is no bug, lets keep things here as is.

I saw 1.07x for the former, but on very different hardware.

I got something like this with your patch.

[hpkfft]

If you're willing to take the risk of breaking abs() on some exotic platform for the huge number of Python developers, you should also take the risk of breaking the C API function _Py_c_abs() on the same exotic platform for, ummm, well, nobody really.
I think there's a reasonable expectation that the C API function returns the same bits as the Python function. I would like to see them kept in sync....

[skirpichev]

I think there's a reasonable expectation that the C API function returns the same bits as the Python function. I would like to see them kept in sync....

It's already out of sync, e.g. for _Py_c_pow() vs complex_pow().

[skirpichev]

I decided to use alternative approach, ignoring errno, as proposed in #156145. This is in a separate commit and can be easily reverted.

Edit: this will benefit from new private API in #156694. So, I would appreciate if that pr will be reviewed/merged first.