FEAT: Implementing `same_value` casting rule in quaddtype #246

SwayamInSync · 2025-12-26T13:13:40Z

closes #153

As per the title

SwayamInSync · 2025-12-26T13:20:16Z

Details for easing the review process

Only casts.cpp and test_quaddtype requires the major review, I added comments wherever I made the changes to give reasoning for that step, the rest files are modified for the caues listed below

Small refactoring

quad_value union with __float128 causes lot of ABI & compiler optimization issues in C++, hence refactoring the methods to not perform any kind of copy of this passed union instead use pointers
Inter-backend operation is broken in previous builds, not sure why someone wants to go in that direction, but just for the sake of completeness I fixed it here as well, to implement the same_value casting between QuadPrecision with different backends
Remove the aligned/unlaigned separate loops with their templated versions

SwayamInSync · 2025-12-26T13:24:38Z

Tagging everyone involved in the prev PR discussions to have a look
@seberg @ngoldbaum @mattip @juntyr

juntyr · 2025-12-26T16:12:00Z

quaddtype/numpy_quaddtype/src/casts.cpp

+
+    if (given_descrs[0]->backend != given_descrs[1]->backend) {
+        // Different backends require actual conversion, no view possible
+        *view_offset = NPY_MIN_INTP;


what does the view_offset mean?

it signals numpy that whether this cast is possible without allocating new memory i.e. return a simple memory view (which in this case is casting to same backends). If memory must need to be allocated (inter-backend casts) then we set this flag to a "special" value, which is here NPY_MIN_INTP (defined as minimum value of long int)

juntyr · 2025-12-26T16:14:18Z

quaddtype/numpy_quaddtype/src/casts.cpp

+    loop_descrs[1] = given_descrs[1];
+
+    if (given_descrs[0]->backend != given_descrs[1]->backend) {
+        // Different backends require actual conversion, no view possible


I thought we stored them in a union so that views would be possible? Do we know why not?

In any case explicit casting is better here since it better communicates that loss may happen.

I know we also export a function to check if long double is float128, if so should we allow the cast from sleef to long double as safe?

Union is a convenient and efficient way to store only one of the values, that does not gurantee that stored bytes will be interpreted in same manner (atleast on systems where __float128 is not defined and longdouble != 128 bits)
On systems Sleef_quad can be __float128 or a struct of 2 int64, the interpretation and handling of bytes will be needed for cross-platform support.

I know we also export a function to check if long double is float128, if so should we allow the cast from sleef to long double as safe?

This is indeed possible but I believe this flag should represent a general scenario, as if somebody on their system has longdouble == 128 bits and they checked and found "oh cast is safe" and they thought its universal as in NumPy we try to keep things supported cross-platform. Although I am against of users performing inter-backend operations.

juntyr · 2025-12-26T16:21:50Z

quaddtype/numpy_quaddtype/src/casts.cpp

+        }
+
+        // Compare in SLEEF domain
+        if (Sleef_iunordq1(in_val->sleef_value, roundtrip.sleef_value))


This doesn't check that both are NaN, just that either is. If we guarantee that NaNs must convert successfully, then just saying that as

if (Sleef_iunordq1(in_val->sleef_value, in_val->sleef_value)) return 1; // NaN input, output guaranteed NaN

would be more explicit

If we don't want to guarantee it (and the long double check doesn't) then we should use

if (Sleef_iunordq1(in_val->sleef_value, in_val->sleef_value) && Sleef_iunordq1(roundtrip.sleef_value, roundtrip.sleef_value))

also handle this in all cases below

Right I forgot this is an OR condition, I'll fix this to assess on based on inputs AND ensuring NAN roundtrip

juntyr · 2025-12-26T16:23:30Z

quaddtype/numpy_quaddtype/src/casts.cpp

+            return 1;  // Both NaN
+        if (Sleef_icmpeqq1(in_val->sleef_value, roundtrip.sleef_value))
+            return 1;  // Equal
+        if (Sleef_icmpeqq1(in_val->sleef_value, QUAD_ZERO) && Sleef_icmpeqq1(roundtrip.sleef_value, QUAD_ZERO))


Why do we have this extra check? If the previous equality check doesn't catch +0.0 == -0.0, then this one would still check (+0.0 == +0.0) && (-0.0 == +0.0) which is no better

also handle this in all cases below

Oh wait, there's another fault I forgot to test the sign preservation in 0 case, its possible the casting itself will be broken.
I'll update it

juntyr · 2025-12-26T16:26:05Z

quaddtype/numpy_quaddtype/src/casts.cpp

    QuadPrecDTypeObject *descr_out = (QuadPrecDTypeObject *)context->descriptors[1];
+    QuadBackendType backend_in = descr_in->backend;
+    QuadBackendType backend_out = descr_out->backend;
+    int same_value_casting = ((context->flags & NPY_SAME_VALUE_CONTEXT_FLAG) == NPY_SAME_VALUE_CONTEXT_FLAG);


can this be a bool?

juntyr · 2025-12-26T16:28:08Z

quaddtype/numpy_quaddtype/src/casts.cpp

+            {
+              long double ld = in_val.longdouble_value;
+              if (std::isnan(ld)) {
+                  out_val.sleef_value = QUAD_PRECISION_NAN;


could we preserve the sign here?

juntyr · 2025-12-26T16:30:16Z

quaddtype/numpy_quaddtype/src/casts.cpp

+    // Compare original and roundtripped values
+    if (backend == BACKEND_SLEEF) {
+        // NaN == NaN for same_value purposes
+        if (Sleef_iunordq1(in_val->sleef_value, roundtrip.sleef_value))


same as above

juntyr · 2025-12-26T16:32:13Z

quaddtype/numpy_quaddtype/src/casts.cpp

    QuadBackendType backend = descr_in->backend;

    npy_intp unicode_size_chars = descrs[1]->elsize / 4;
+    int same_value_casting = ((context->flags & NPY_SAME_VALUE_CONTEXT_FLAG) == NPY_SAME_VALUE_CONTEXT_FLAG);


same as above

juntyr · 2025-12-26T16:34:25Z

quaddtype/numpy_quaddtype/src/casts.cpp

+    quad_value roundtrip = to_quad<T>(*y, backend);
+    if(backend == BACKEND_SLEEF) 
+    {
+        if(Sleef_iunordq1(x->sleef_value, roundtrip.sleef_value))


same as above

juntyr · 2025-12-26T16:36:20Z

quaddtype/numpy_quaddtype/src/scalar_ops.cpp

+            // we could allow, but this will be bad
+            // Two values that are different in quad precision, 
+            // might appear equal when converted to double.


Suggested change

// we could allow, but this will be bad

// Two values that are different in quad precision,

// might appear equal when converted to double.

// we could allow this, but it would be bad

// since two values that are different in quad precision

// might appear equal when converted to double.

juntyr · 2025-12-26T16:37:11Z

quaddtype/numpy_quaddtype/src/utilities.c

    }
    else {
-        return Sleef_cast_from_doubleq1(in_val->longdouble_value);
+        return Sleef_cast_from_doubleq1((double)(in_val->longdouble_value));


we use static_cast elsewhere?

juntyr · 2025-12-26T16:37:48Z

quaddtype/tests/test_quaddtype.py

+])
+@pytest.mark.parametrize("value", [
+    "0.0", "-0.0", "1.0", "-1.0", "3.14159265358979323846",
+    "inf", "-inf", "nan", "1e100", "1e-100",


please add -nan case as well and check that the sign is preserved

also check that the sign is preserved for -0.0 and +0.0

juntyr · 2025-12-26T16:39:31Z

quaddtype/tests/test_quaddtype.py

+    @pytest.mark.parametrize("dtype", [
+        np.float16, np.float32, np.float64, np.longdouble
+    ])
+    @pytest.mark.parametrize("val", [0.0, -0.0, float('inf'), float('-inf'), float('nan')])


please add -nan and check that the sign is preserved, same for +0.0 and -0.0

juntyr · 2025-12-26T16:40:45Z

quaddtype/tests/test_quaddtype.py

+        values = [
+            "0.0", "-0.0", "1.0", "-1.0",
+            "3.14159265358979323846264338327950288",  # pi with full quad precision
+            "inf", "-inf", "nan",


what happens for -nan

juntyr · 2025-12-26T16:41:06Z

quaddtype/tests/test_quaddtype.py

+    def test_same_value_cast_strings_enough_width(self, dtype):
+        """Test that string types with enough width can represent quad values exactly."""
+        values = [
+            "0.0", "-0.0", "1.0", "-1.0",


can we also check the sign for +0.0 and -0.0?

juntyr · 2025-12-26T16:41:26Z

quaddtype/tests/test_quaddtype.py

+    def test_same_value_cast_strings_narrow_width(self, dtype):
+        """Test that string types with narrow width fail for values that need more precision."""
+        # Values that can fit in 10 chars should pass
+        passing_values = ["0.0", "1.0", "-1.0", "inf", "-inf", "nan"]


also check -0.0 and -nan

juntyr · 2025-12-26T16:41:48Z

quaddtype/tests/test_quaddtype.py

+            0.0, -0.0, 1.0, -1.0,
+            0.5, 0.25, 0.125,
+            2.0, 4.0, 8.0,
+            "inf", "-inf", "nan",


also -nan and sign checks for nan and 0.0

juntyr

Thanks @SwayamInSync for your work! My comments are mostly nits and extra tests, I'll review again afterwards

SwayamInSync added 30 commits December 8, 2025 14:41

updating TARGET_VERSION and numpy_to_quad resove desc

fe04b3b

Merge branch 'main' into same-value

cdfe02c

Merge branch 'main' into same-value

9e3afc3

Merge branch 'main' into same-value

605aa59

quad2quad

9b4d83d

fix heisenbugs

95a253a

refactor aligned/unaligned into templates

277ee7b

resolve desc + quad2numpy loop fix

5cb4e4a

adding same_value int tests

3382565

handling nan in same_value

534a64e

fix tests

f696848

again union hesinbug?

22327f8

just match with valueerror

6fa020d

use memcpy

0babf9d

use memcmp

a5cf124

switch back to no union

90b824d

addded float tests

ff69b8e

use double's tiny in ld

30f6b95

adding quad->str same_vale

5c5d791

improve error msg

dde4a84

make all from_quad uses const pointer to union

8862c23

fixed string same_value

e374a36

use quad2sleefquad

c458d53

remove non-native order tets for respective systems

9d10144

powerpc has ld as quad

c17c3d0

memory barrier

ae9986b

will cont tomorrow from here

308f136

quad2quad same_value

00acaca

nolong need pyucs path

e68e6be

remove unused apis

3f0cf90

SwayamInSync added this to the v1.0 milestone Dec 26, 2025

SwayamInSync added the numpy_quaddtype label Dec 26, 2025

juntyr reviewed Dec 26, 2025

View reviewed changes

juntyr suggested changes Dec 26, 2025

View reviewed changes

Uh oh!

FEAT: Implementing same_value casting rule in quaddtype #246

Are you sure you want to change the base?

FEAT: Implementing same_value casting rule in quaddtype #246

Uh oh!

Conversation

SwayamInSync commented Dec 26, 2025

Uh oh!

SwayamInSync commented Dec 26, 2025

Details for easing the review process

Small refactoring

Uh oh!

SwayamInSync commented Dec 26, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

juntyr left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FEAT: Implementing `same_value` casting rule in quaddtype #246

FEAT: Implementing `same_value` casting rule in quaddtype #246