8000 Updating test_math.py to CPython 3.12.9 by hbina · Pull Request #5507 · RustPython/RustPython · GitHub
[go: up one dir, main page]

Skip to content

Updating test_math.py to CPython 3.12.9 #5507

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 20, 2025

Conversation

hbina
Copy link
Contributor
@hbina hbina commented Feb 7, 2025

Still work in progress..commits will get messy until I fix everything.

Edit: Part of #5104

@hbina
Copy link
Contributor Author
hbina commented Feb 7, 2025

One of the failing test involves fsum. The implementation in CPython is like this

        if (x != 0.0) {
            if (! Py_IS_FINITE(x)) {
                /* a nonfinite x could arise either as
                   a result of intermediate overflow, or
                   as a result of a nan or inf in the
                   summands */
                if (Py_IS_FINITE(xsave)) {
                    PyErr_SetString(PyExc_OverflowError,
                          "intermediate overflow in fsum");
                    goto _fsum_error;
                }
                if (Py_IS_INFINITY(xsave))
                    inf_sum += xsave;
                special_sum += xsave;
                /* reset partials */
                n = 0;
            }
            else if (n >= m && _fsum_realloc(&p, n, ps, &m))
                goto _fsum_error;
            else
                p[n++] = x;
        }

I wonder why there's a special case if x + y is an INF, why not just collapse the partials into the inf_sum?

@hbina
Copy link
Contributor Author
hbina commented Feb 7, 2025

Oh dear..steps in math.nextafter is kinda crazy

steps = PyNumber_Index(steps);
    if (steps == NULL) {
        return NULL;
    }
    assert(PyLong_CheckExact(steps));
    if (_PyLong_IsNegative((PyLongObject *)steps)) {
        PyErr_SetString(PyExc_ValueError,
                        "steps must be a non-negative integer");
        Py_DECREF(steps);
        return NULL;
    }

    unsigned long long usteps_ull = PyLong_AsUnsignedLongLong(steps);
    // Conveniently, uint64_t and double have the same number of bits
    // on all the platforms we care about.
    // So if an overflow occurs, we can just use UINT64_MAX.
    Py_DECREF(steps);
    if (usteps_ull >= UINT64_MAX) {
        // This branch includes the case where an error occurred, since
        // (unsigned long long)(-1) = ULLONG_MAX >= UINT64_MAX. Note that
        // usteps_ull can be strictly larger than UINT64_MAX on a machine
        // where unsigned long long has width > 64 bits.
        if (PyErr_Occurred()) {
            if (PyErr_ExceptionMatches(PyExc_OverflowError)) {
                PyErr_Clear();
            }
            else {
                return NULL;
            }
        }
        usteps_ull = UINT64_MAX;
    }
    assert(usteps_ull <= UINT64_MAX);
    uint64_t usteps = (uint64_t)usteps_ull;

    if (usteps == 0) {
        return PyFloat_FromDouble(x);
    }
    if (Py_IS_NAN(x)) {
        return PyFloat_FromDouble(x);
    }
    if (Py_IS_NAN(y)) {
        return PyFloat_FromDouble(y);
    }

    // We assume that double and uint64_t have the same endianness.
    // This is not guaranteed by the C-standard, but it is true for
    // all platforms we care about. (The most likely form of violation
    // would be a "mixed-endian" double.)
    union pun {double f; uint64_t i;};
    union pun ux = {x}, uy = {y};
    if (ux.i == uy.i) {
        return PyFloat_FromDouble(x);
    }

    const uint64_t sign_bit = 1ULL<<63;

    uint64_t ax = ux.i & ~sign_bit;
    uint64_t ay = uy.i & ~sign_bit;

    // opposite signs
    if (((ux.i ^ uy.i) & sign_bit)) {
        // NOTE: ax + ay can never overflow, because their most significant bit
        // ain't set.
        if (ax + ay <= usteps) {
            return PyFloat_FromDouble(uy.f);
        // This comparison has to use <, because <= would get +0.0 vs -0.0
        // wrong.
        } else if (ax < usteps) {
            union pun result = {.i = (uy.i & sign_bit) | (usteps - ax)};
            return PyFloat_FromDouble(result.f);
        } else {
            ux.i -= usteps;
            return PyFloat_FromDouble(ux.f);
        }
    // same sign
    } else if (ax > ay) {
        if (ax - ay >= usteps) {
            ux.i -= usteps;
            return PyFloat_FromDouble(ux.f);
        } else {
            return PyFloat_FromDouble(uy.f);
        }
    } else {
        if (ay - ax >= usteps) {
            ux.i += usteps;
            return PyFloat_FromDouble(ux.f);
        } else {
            return PyFloat_FromDouble(uy.f);
        }
    }

@hbina
Copy link
Contributor Author
hbina commented Feb 8, 2025

Now that I've read up on floating points again, nextafter is not so bad after all.
One thing that I don't understand is why this is using | and not +?

    // opposite signs
    if (((ux.i ^ uy.i) & sign_bit)) {
        // NOTE: ax + ay can never overflow, because their most significant bit
        // ain't set.
        if (ax + ay <= usteps) {
            return PyFloat_FromDouble(uy.f);
        // This comparison has to use <, because <= would get +0.0 vs -0.0
        // wrong.
        } else if (ax < usteps) {
            union pun result = {.i = (uy.i & sign_bit) | (usteps - ax)}; // <---this | here
            return PyFloat_FromDouble(result.f);
        } else {
            ux.i -= usteps;
            return PyFloat_FromDouble(ux.f);
        }

Edit: Nevermind...I understand it now. I thought the LHS there is doing uy.i & !sign_bit but it is actually just getting the sign bit, and then just use the diff (usteps - ax) as the value.

@hbina hbina force-pushed the hbina-fix-test-math branch from 4f8d25b to b0f87b1 Compare February 9, 2025 06:19
Copy link
Member
@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!
I have no idea about fsum details.

Signed-off-by: Hanif Ariffin <hanif.ariffin.4326@gmail.com>
@hbina hbina force-pushed the hbina-fix-test-math branch from b0f87b1 to 60805fb Compare February 12, 2025 05:49
@hbina hbina marked this pull request as ready for review February 15, 2025 00:39
Copy link
Member
@youknowone youknowone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much!

vm.new_value_error("steps must be a non-negative integer".to_string())
);
}
Ok(float_ops::nextafter(*arg.x, *arg.y, Some(steps as u64)))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since nextafter call with step and without step are well separated, float_ops::nextafter also looks to be fine to be separated.
Because nextafter is originated from libc, I prefer to keep it as it is unless there is a good reason to change it.

@youknowone youknowone merged commit 65dcf1c into RustPython:main Feb 20, 2025
11 checks passed
@hbina hbina deleted the hbina-fix-test-math branch February 20, 2025 10:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0