-
-
Notifications
You must be signed in to change notification settings - Fork 6
Adding __reduce__ method to QuadPrecision for supporting pickle
#105
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
base: main
Are you sure you want to change the base?
Changes from all commits
5b69baa
9cd5674
4c32b91
34c9c6d
9d57423
e328d11
1e69bec
85074c1
f0b4405
7574ada
ee40e6d
a574c02
a2b59fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1929,7 +1929,7 @@ Dragon4_PrintFloat_Sleef_quad(Sleef_quad *value, Dragon4_Options *opt) | |
| /* mantissa_lo is unchanged */ | ||
| exponent = floatExponent - 16383 - 112; | ||
| mantissaBit = 112; | ||
| hasUnequalMargins = (floatExponent != 1) && (mantissa_hi == 0 && mantissa_lo == 0); | ||
| hasUnequalMargins = (floatExponent != 1) && (mantissa_hi == (1ull << 48) && mantissa_lo == 0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this get fixed in NumPy yet?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not yet, I can drop a fix there as well later (or globally replacing numpy's ldbl with quaddtype will auto fix it) |
||
| } | ||
| else { | ||
| /* subnormal */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,8 @@ | |
| #include <sleefquad.h> | ||
| #include <stdlib.h> | ||
| #include <math.h> | ||
| #include <string.h> | ||
| #include <float.h> | ||
|
|
||
| #define PY_ARRAY_UNIQUE_SYMBOL QuadPrecType_ARRAY_API | ||
| #define NPY_NO_DEPRECATED_API NPY_2_0_API_VERSION | ||
|
|
@@ -40,6 +42,8 @@ QuadPrecision_raw_new(QuadBackendType backend) | |
| new->value.sleef_value = Sleef_cast_from_doubleq1(0.0); | ||
| } | ||
| else { | ||
| // An 80-bit long double occupies 16 bytes but writes only 10; | ||
| memset(&new->value, 0, sizeof(new->value)); | ||
| new->value.longdouble_value = 0.0L; | ||
| } | ||
| return new; | ||
|
|
@@ -335,19 +339,6 @@ QuadPrecision_str_dragon4(QuadPrecisionObject *self) | |
| } | ||
| } | ||
|
|
||
| static PyObject * | ||
| QuadPrecision_str(QuadPrecisionObject *self) | ||
| { | ||
| char buffer[128]; | ||
| if (self->backend == BACKEND_SLEEF) { | ||
| Sleef_snprintf(buffer, sizeof(buffer), "%.*Qe", SLEEF_QUAD_DIG, self->value.sleef_value); | ||
| } | ||
| else { | ||
| snprintf(buffer, sizeof(buffer), "%.35Le", self->value.longdouble_value); | ||
| } | ||
| return PyUnicode_FromString(buffer); | ||
| } | ||
|
|
||
| static PyObject * | ||
| QuadPrecision_repr_dragon4(QuadPrecisionObject *self) | ||
| { | ||
|
|
@@ -358,7 +349,7 @@ QuadPrecision_repr_dragon4(QuadPrecisionObject *self) | |
| .sign = 1, | ||
| .trim_mode = TrimMode_LeaveOneZero, | ||
| .digits_left = 1, | ||
| .exp_digits = 3}; | ||
| .exp_digits = 4}; | ||
|
|
||
| PyObject *str; | ||
| if (self->backend == BACKEND_SLEEF) { | ||
|
|
@@ -633,11 +624,157 @@ QuadPrecision_as_integer_ratio(QuadPrecisionObject *self, PyObject *Py_UNUSED(ig | |
| return PyTuple_Pack(2, numerator, denominator); | ||
| } | ||
|
|
||
| static int | ||
| quad_host_is_big_endian(void) | ||
| { | ||
| uint16_t probe = 1; | ||
| return ((const unsigned char *)&probe)[0] == 0; | ||
| } | ||
|
|
||
|
|
||
| static void | ||
| quad_copy_canonical(unsigned char *dst, const unsigned char *src, size_t n) | ||
| { | ||
| if (quad_host_is_big_endian()) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. re your question: I'd keep this. It makes sense to use native endianness. Maybe because reconstruction depends on endianness, you should also write the byte order to the pickle file?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Writing byte-order might not be needed as during the unpickle it automatically byteswap as per the platform-endianness (no corruption) pickle file always stores in little-endian |
||
| for (size_t i = 0; i < n; i++) { | ||
| dst[i] = src[n - 1 - i]; | ||
| } | ||
| } | ||
| else { | ||
| memcpy(dst, src, n); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| static PyObject * | ||
| QuadPrecision_reduce(QuadPrecisionObject *self, PyObject *Py_UNUSED(ignored)) | ||
| { | ||
| size_t nbytes = (self->backend == BACKEND_SLEEF) | ||
| ? sizeof(self->value.sleef_value) | ||
| : sizeof(self->value.longdouble_value); | ||
| const unsigned char *src = (self->backend == BACKEND_SLEEF) | ||
| ? (const unsigned char *)&self->value.sleef_value | ||
| : (const unsigned char *)&self->value.longdouble_value; | ||
|
|
||
| unsigned char raw[sizeof(quad_value)]; | ||
| quad_copy_canonical(raw, src, nbytes); | ||
|
Comment on lines
+652
to
+660
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this leaves the last six bytes of the value uninitialized for systems with 80-bit longdouble. Even though there are only 80 significant bits, the type is still 16 bytes wide. I think you'd avoid this issue if you added a line here that does
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch, fixed. |
||
|
|
||
| PyObject *data = PyBytes_FromStringAndSize((const char *)raw, (Py_ssize_t)nbytes); | ||
| if (data == NULL) { | ||
| return NULL; | ||
| } | ||
|
|
||
| PyObject *backend_obj = PyUnicode_FromString( | ||
| self->backend == BACKEND_SLEEF ? "sleef" : "longdouble"); | ||
| if (backend_obj == NULL) { | ||
| Py_DECREF(data); | ||
| return NULL; | ||
| } | ||
|
|
||
| PyObject *module = PyImport_ImportModule("numpy_quaddtype._quaddtype_main"); | ||
| if (module == NULL) { | ||
| Py_DECREF(data); | ||
| Py_DECREF(backend_obj); | ||
| return NULL; | ||
| } | ||
| PyObject *reconstruct = PyObject_GetAttrString(module, "from_raw_bytes"); | ||
| Py_DECREF(module); | ||
| if (reconstruct == NULL) { | ||
| Py_DECREF(data); | ||
| Py_DECREF(backend_obj); | ||
| return NULL; | ||
| } | ||
|
|
||
| PyObject *args; | ||
| if (self->backend == BACKEND_LONGDOUBLE) { | ||
| // Tag longdouble payloads with the platform's LDBL_MANT_DIG | ||
| PyObject *fmt = PyLong_FromLong((long)LDBL_MANT_DIG); | ||
| if (fmt == NULL) { | ||
| Py_DECREF(data); | ||
| Py_DECREF(backend_obj); | ||
| Py_DECREF(reconstruct); | ||
| return NULL; | ||
| } | ||
| args = PyTuple_Pack(3, data, backend_obj, fmt); | ||
| Py_DECREF(fmt); | ||
| } | ||
| else { | ||
| args = PyTuple_Pack(2, data, backend_obj); | ||
| } | ||
| Py_DECREF(data); | ||
| Py_DECREF(backend_obj); | ||
| if (args == NULL) { | ||
| Py_DECREF(reconstruct); | ||
| return NULL; | ||
| } | ||
|
|
||
| PyObject *result = PyTuple_Pack(2, reconstruct, args); | ||
| Py_DECREF(reconstruct); | ||
| Py_DECREF(args); | ||
| return result; | ||
| } | ||
|
|
||
| PyObject * | ||
| QuadPrecision_from_raw_bytes(PyObject *Py_UNUSED(module), PyObject *args) | ||
| { | ||
| Py_buffer view; | ||
| const char *backend_str = "sleef"; | ||
| int ld_format = -1; // source LDBL_MANT_DIG for longdouble; -1 = not provided | ||
| if (!PyArg_ParseTuple(args, "y*|si", &view, &backend_str, &ld_format)) { | ||
| return NULL; | ||
| } | ||
|
|
||
| QuadBackendType backend = BACKEND_SLEEF; | ||
| if (strcmp(backend_str, "longdouble") == 0) { | ||
| backend = BACKEND_LONGDOUBLE; | ||
| } | ||
| else if (strcmp(backend_str, "sleef") != 0) { | ||
| PyBuffer_Release(&view); | ||
| PyErr_SetString(PyExc_ValueError, "Invalid backend. Use 'sleef' or 'longdouble'."); | ||
| return NULL; | ||
| } | ||
|
|
||
| size_t expected = (backend == BACKEND_SLEEF) ? sizeof(Sleef_quad) : sizeof(long double); | ||
| if (view.len != (Py_ssize_t)expected) { | ||
| PyErr_Format(PyExc_ValueError, | ||
| "QuadPrecision.from_raw_bytes expected %zu bytes for the '%s' " | ||
| "backend, got %zd", | ||
| expected, backend_str, view.len); | ||
| PyBuffer_Release(&view); | ||
| return NULL; | ||
| } | ||
|
Comment on lines
+738
to
+745
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on e.g. x86_64 glibc sizeof(long double) is 16, which is the same size as sleef_quad. That means this won't catch format mismatches (I think?), it will only catch data corruption. Even if the check did work, it still couldn't distinguish between different long double formats (e.g. 80-bit x87 vs true 128-bit on ppc64le). Maybe it makes sense to store some kind tag that indicates what kind of long double data are in the file? Pickles do get shared between machines, so it's not out of the realm of possibility for this to cause issues.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fixed |
||
|
|
||
| if (backend == BACKEND_LONGDOUBLE && ld_format != -1 && | ||
| ld_format != LDBL_MANT_DIG) { | ||
| PyErr_Format(PyExc_ValueError, | ||
| "Cannot unpickle a 'longdouble' QuadPrecision created on a " | ||
| "platform with a different long double format " | ||
| "(LDBL_MANT_DIG=%d) than this one (LDBL_MANT_DIG=%d).", | ||
| ld_format, (int)LDBL_MANT_DIG); | ||
| PyBuffer_Release(&view); | ||
| return NULL; | ||
| } | ||
|
|
||
| QuadPrecisionObject *self = QuadPrecision_raw_new(backend); | ||
| if (self == NULL) { | ||
| PyBuffer_Release(&view); | ||
| return NULL; | ||
| } | ||
| unsigned char *dst = (backend == BACKEND_SLEEF) | ||
| ? (unsigned char *)&self->value.sleef_value | ||
| : (unsigned char *)&self->value.longdouble_value; | ||
| quad_copy_canonical(dst, (const unsigned char *)view.buf, expected); | ||
| PyBuffer_Release(&view); | ||
| return (PyObject *)self; | ||
| } | ||
|
|
||
| static PyMethodDef QuadPrecision_methods[] = { | ||
| {"is_integer", (PyCFunction)QuadPrecision_is_integer, METH_NOARGS, | ||
| "Return True if the value is an integer."}, | ||
| {"as_integer_ratio", (PyCFunction)QuadPrecision_as_integer_ratio, METH_NOARGS, | ||
| "Return a pair of integers whose ratio is exactly equal to the original value."}, | ||
| {"__reduce__", (PyCFunction)QuadPrecision_reduce, METH_NOARGS, | ||
| "Support pickling: return (from_raw_bytes, (raw_bytes, backend[, ld_format]))."}, | ||
| {NULL, NULL, 0, NULL} /* Sentinel */ | ||
| }; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.