gh-121149: improve accuracy of builtin sum() for complex inputs

[skirpichev]

• Issue: gh-121149

---

📚 Documentation preview 📚: https://cpython-previews--121176.org.readthedocs.build/

[skirpichev]

item = PyIter_Next(iter); has an overhead

That's out of the scope for this pr.

[rhettinger]

Overall this patch looks basically sound.

If you're open to change, I wish it was in more of a functional style than a mutate in place style. I find the latter harder to read and harder to debug.

Also the name csum is meaningful to you because you already know that you're implementing a compensated summation, but to fresh eyes, it is hard to not read csum as complex_sum which is incorrect.

I suggest something like this:

typedef struct {
    double hi;     /* high-order bits for a running sum */
    double lo;     /* a running compensation for lost low-order bits */
} CompensatedSum;

static inline CompensatedSum
cs_from_double(double x)
{
    return (CompensatedSum) {x, 0.0};
}

static inline CompensatedSum
cs_add(CompensatedSum total, double x)
{
    double t = total.hi + x;
    if (fabs(total.hi) >= fabs(x)) {
        c = (total.hi - t) + x;
    } else {
        c = (x - t) + total.hi;
    }
    return (CompensatedSum) {t, total.lo + c}
}

static inline double
cs_to_double(CompensatedSum total)
{
    if (total.lo && isfinite(total.lo))
        return total.hi + total.lo;
    return total.hi;
}

It could be used very plainly and readably, making it obvious what is changing:

CompensatedSum total = cs_from_double(start);
for (i=0 ; i<n ; i++) {
    total = cs_add(total, xarr[i]);
}
return cs_to_double(total);

I'm not attached to the names only the core concept. Use sum and c if you prefer those to hi and lo.

[skirpichev]

I suggest something like this: ... CompensatedSum

Thanks, I like new names.

Use sum and c if you prefer those to hi and lo.

That was chosen for compatibility with the referenced wiki page, but the later variant does make sense too. And it's already used in the fsum().

2698be63ef - renaming part

I wish it was in more of a functional style than a mutate in place style. I find the latter harder to read and harder to debug.

Does make sense. I did this change in the second commit, 5242bd6715 (with a minor change: don't introduce temporary c variable, that has performance impact).

As we inline all helpers - this variant should have same performance as in the commit above.