Skip to content

BUG: Fixing reduction logical ufuncs #220

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.

Sign up for GitHub

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

BUG: Fixing reduction logical ufuncs #220

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
35d4d93
comparison reduction ops support
SwayamInSync Oct 30, 2025
e3c51a9
np.f64
SwayamInSync Oct 30, 2025
41fc5ad
better comment + getattr
SwayamInSync Oct 30, 2025
5456c93
looooop
SwayamInSync Nov 7, 2025
70f5fdc
no quad in rd
SwayamInSync Nov 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions quaddtype/numpy_quaddtype/src/casts.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -502,6 +502,8 @@ template <>
inline npy_byte
from_quad<npy_byte>(quad_value x, QuadBackendType backend)
{
// reduction ops often give warning, we can handle the NAN casting
Copy link
Contributor

@juntyr juntyr Oct 30, 2025

Choose a reason for hiding this comment

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

I don't really understand this comment

Copy link
Member Author

@SwayamInSync SwayamInSync Oct 30, 2025

Choose a reason for hiding this comment

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

I should improve this comment, basically saying the runtime warnings we used to get often casting is the reason.
Casting from quad NaN or inf to double, etc Instead if we just catch and return for example in this case if we catch NaN and return NP_True then there will be no warnings otherwise that part will go through casting and leads to warnings.

I tested this but revert to original with just comment. Often different ops use casting in one way or another so when fixing that, knowing this information might be helpful

// this behaviour might apply to all casting
if (backend == BACKEND_SLEEF) {
return (npy_byte)Sleef_cast_to_int64q1(x.sleef_value);
}
Expand Down
2 changes: 1 addition & 1 deletion quaddtype/numpy_quaddtype/src/umath/comparison_ops.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ create_quad_comparison_ufunc(PyObject *numpy, const char *ufunc_name)
return -1;
}

PyObject *DTypes = PyTuple_Pack(3, &PyArrayDescr_Type, &PyArrayDescr_Type, &PyArray_BoolDType);
PyObject *DTypes = PyTuple_Pack(3, Py_None, Py_None, Py_None);
Copy link
Member

@ngoldbaum ngoldbaum Oct 30, 2025

Choose a reason for hiding this comment

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

@seberg this has performance implications, right?

Copy link
Member

@seberg seberg Oct 30, 2025

Choose a reason for hiding this comment

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

It doesn't change things really. The problem with this (but also the old version) is mostly that it doesn't define an order...
... and I am not sure what happens if another DType wants to do this as well, they might clash (depending on who is faster).

In general, I think it may be best to just not worry about performance at all, since we cache promotion results. The problem then being just that there isn't really a well defined order for multiple similar promoters (which is probably fine in practice, except that I think NumPy rejects it for now).

(I.e. maybe for promoters it should be fine to say "yeah, just try it no matter what DTypes come in")

Copy link
Member

@seberg seberg Oct 30, 2025

Choose a reason for hiding this comment

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

@SwayamInSync ideally, you probably just register it twice, once with the first being quaddtype and once with the second.
Maybe we should explicitly allow this (for multiple dtypes, but then it has undefined order), though.

Copy link
Member Author

@SwayamInSync SwayamInSync Oct 31, 2025

Choose a reason for hiding this comment

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

ideally, you probably just register it twice

You mean registering promoter twice? I tried this regiserting the loop, it worked but undo because of code duplication.
I wanted the promoter to handle this stuff

if (DTypes == 0) {
Py_DECREF(promoter_capsule);
return -1;
Expand Down
64 changes: 64 additions & 0 deletions quaddtype/tests/test_quaddtype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,70 @@ def test_array_minmax(op, a, b):
assert np.signbit(float_res) == np.signbit(
quad_res), f"Zero sign mismatch for {op}({a}, {b})"

class TestComparisonReductionOps:
"""Test suite for comparison reduction operations on QuadPrecision arrays."""

@pytest.mark.parametrize("op", ["all", "any"])
@pytest.mark.parametrize("input_array", [
(["1.0", "2.0", "3.0"]),
(["1.0", "0.0", "3.0"]),
(["0.0", "0.0", "0.0"]),
# Including negative zero
(["-0.0", "0.0"]),
# Including NaN (should be treated as true)
(["nan", "1.0"]),
(["nan", "0.0"]),
(["nan", "nan"]),
# inf cases
(["inf", "1.0"]),
(["-inf", "0.0"]),
(["inf", "-inf"]),
# Mixed cases
(["1.0", "-0.0", "nan", "inf"]),
(["0.0", "-0.0", "nan", "-inf"]),
])
def test_reduction_ops(self, op, input_array):
"""Test all and any reduction operations."""
quad_array = np.array([QuadPrecision(x) for x in input_array])
float_array = np.array([float(x) for x in input_array])
if op == "all":
result = np.all(quad_array)
expected = np.all(float_array)
else: # op == "any"
result = np.any(quad_array)
expected = np.any(float_array)

assert result == expected, (
f"Reduction op '{op}' failed for input {input_array}: "
f"expected {expected}, got {result}"
)

@pytest.mark.parametrize("val_str", [
"0.0",
"-0.0",
"1.0",
"-1.0",
"nan",
"inf",
"-inf",
])
def test_scalar_reduction_ops(self, val_str):
"""Test reduction operations on scalar QuadPrecision values."""
quad_val = QuadPrecision(val_str)
float_val = np.float64(val_str)

result_all = quad_val.all()
expected_all_result = float_val.all()
assert result_all == expected_all_result, (
f"Scalar all failed for {val_str}: expected {expected_all_result}, got {result_all}"
)

result_any = quad_val.any()
expected_any_result = float_val.any()
assert result_any == expected_any_result, (
f"Scalar any failed for {val_str}: expected {expected_any_result}, got {result_any}"
)


# Logical operations tests
@pytest.mark.parametrize("op", ["logical_and", "logical_or", "logical_xor"])
Expand Down