Updating test_math.py to CPython 3.12.9#5507
Merged
youknowone merged 3 commits intoRustPython:mainfrom Feb 20, 2025
Merged
Conversation
Contributor
Author
|
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 |
Contributor
Author
|
Oh dear.. 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);
}
} |
Contributor
Author
|
Now that I've read up on floating points again, nextafter is not so bad after all. // 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 |
4f8d25b to
b0f87b1
Compare
youknowone
reviewed
Feb 10, 2025
Member
There was a problem hiding this comment.
Thanks!
I have no idea about fsum details.
Signed-off-by: Hanif Ariffin <hanif.ariffin.4326@gmail.com>
b0f87b1 to
60805fb
Compare
youknowone
approved these changes
Feb 20, 2025
stdlib/src/math.rs
Outdated
| 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))) |
Member
There was a problem hiding this comment.
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.
ded6392 to
6d3960b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Still work in progress..commits will get messy until I fix everything.
Edit: Part of #5104