-
-
Notifications
You must be signed in to change notification settings - Fork 6
BUG: Preserve object common-dtype promotion in Quad ufuncs #118
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
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 |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
|
|
||
| #include <Python.h> | ||
| #include <cstdio> | ||
| #include <cstring> | ||
|
|
||
| #include "numpy/arrayobject.h" | ||
| #include "numpy/ufuncobject.h" | ||
|
|
@@ -256,6 +257,14 @@ quad_reduce_comp_strided_loop_unaligned(PyArrayMethod_Context *context, char *co | |
| } | ||
|
|
||
|
|
||
| static bool | ||
| comparison_object_output_is_bool(PyUFuncObject *ufunc) | ||
| { | ||
| return strcmp(ufunc->name, "logical_and") != 0 && | ||
| strcmp(ufunc->name, "logical_or") != 0 && | ||
| strcmp(ufunc->name, "logical_xor") != 0; | ||
| } | ||
|
|
||
| NPY_NO_EXPORT int | ||
| comparison_ufunc_promoter(PyObject *ufunc_obj, PyArray_DTypeMeta *const op_dtypes[], | ||
| PyArray_DTypeMeta *const signature[], PyArray_DTypeMeta *new_op_dtypes[]) | ||
|
|
@@ -271,20 +280,24 @@ comparison_ufunc_promoter(PyObject *ufunc_obj, PyArray_DTypeMeta *const op_dtype | |
| return 0; | ||
| } | ||
|
|
||
| PyUFuncObject *ufunc = (PyUFuncObject *)ufunc_obj; | ||
| if (quad_ufunc_has_object_input(ufunc, op_dtypes)) { | ||
| for (int i = 0; i < 2; i++) { | ||
| quad_set_promoted_dtype(signature[i], &PyArray_ObjectDType, &new_op_dtypes[i]); | ||
| } | ||
| PyArray_DTypeMeta *output_dtype = comparison_object_output_is_bool(ufunc) | ||
| ? &PyArray_BoolDType | ||
| : &PyArray_ObjectDType; | ||
| quad_set_promoted_dtype(signature[2], output_dtype, &new_op_dtypes[2]); | ||
| return 0; | ||
| } | ||
|
|
||
| // Normal path: promote both inputs to QuadPrecDType, output is Bool | ||
| for (int i = 0; i < 2; i++) { | ||
| if (signature[i]) { | ||
| Py_INCREF(signature[i]); | ||
| new_op_dtypes[i] = signature[i]; | ||
| } | ||
| else { | ||
| Py_INCREF(&QuadPrecDType); | ||
| new_op_dtypes[i] = &QuadPrecDType; | ||
| } | ||
| quad_set_promoted_dtype(signature[i], &QuadPrecDType, &new_op_dtypes[i]); | ||
| } | ||
|
|
||
| Py_INCREF(&PyArray_BoolDType); | ||
| new_op_dtypes[2] = &PyArray_BoolDType; | ||
| quad_set_promoted_dtype(signature[2], &PyArray_BoolDType, &new_op_dtypes[2]); | ||
| return 0; | ||
| } | ||
|
|
||
|
|
@@ -357,7 +370,7 @@ create_quad_comparison_ufunc(PyObject *numpy, const char *ufunc_name) | |
|
|
||
| // Register promoter for (QuadPrecDType, Any, Bool) - needed for mixed-type comparisons | ||
| PyObject *DTypes = PyTuple_Pack(3, &QuadPrecDType, &PyArrayDescr_Type, &PyArray_BoolDType); | ||
| if (DTypes == 0) { | ||
| if (DTypes == NULL) { | ||
| Py_DECREF(promoter_capsule); | ||
| Py_DECREF(ufunc); | ||
| return -1; | ||
|
|
@@ -373,7 +386,7 @@ create_quad_comparison_ufunc(PyObject *numpy, const char *ufunc_name) | |
|
|
||
| // Register promoter for (Any, QuadPrecDType, Bool) - needed for reverse mixed-type comparisons | ||
| DTypes = PyTuple_Pack(3, &PyArrayDescr_Type, &QuadPrecDType, &PyArray_BoolDType); | ||
|
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. should this one and the one on line 373 use The way you have it written here with exactly Bool as the output doesn't match that. Also use |
||
| if (DTypes == 0) { | ||
| if (DTypes == NULL) { | ||
| Py_DECREF(promoter_capsule); | ||
| Py_DECREF(ufunc); | ||
| return -1; | ||
|
|
@@ -387,6 +400,7 @@ create_quad_comparison_ufunc(PyObject *numpy, const char *ufunc_name) | |
| } | ||
| Py_DECREF(promoter_capsule); | ||
| Py_DECREF(DTypes); | ||
|
|
||
| Py_DECREF(ufunc); | ||
|
|
||
| return 0; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6511,6 +6511,131 @@ def test_divmod_float64_preserves_dtype(self): | |
| assert r.dtype == np.float64 | ||
|
|
||
|
|
||
| class TestObjectPromotion: | ||
| @pytest.fixture | ||
| def backend(self, request): | ||
| return request.param | ||
|
|
||
| @pytest.fixture | ||
| def operands(self, backend): | ||
| quad = np.array([1, 2], dtype=QuadPrecDType(backend=backend)) | ||
| objects = np.array([3, 4], dtype=object) | ||
| return quad, objects | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("op", [np.add, np.multiply]) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_arithmetic_uses_object_loop(self, operands, op, reverse): | ||
| quad, objects = operands | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| result = op(left, right) | ||
| expected = op(left.astype(object), right.astype(object)) | ||
|
|
||
| assert result.dtype == np.dtype(object) | ||
| np.testing.assert_array_equal(result, expected, strict=True) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("op", [np.equal, np.less]) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_comparison_uses_object_loop(self, operands, op, reverse): | ||
| quad, objects = operands | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| result = op(left, right) | ||
| expected = op(left.astype(object), right.astype(object)) | ||
|
|
||
| assert result.dtype == np.dtype(np.bool_) | ||
| np.testing.assert_array_equal(result, expected, strict=True) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("op", [np.logical_and, np.logical_or]) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_logical_uses_object_loop(self, operands, op, reverse): | ||
| quad, objects = operands | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| result = op(left, right) | ||
| expected = op(left.astype(object), right.astype(object)) | ||
|
|
||
| assert result.dtype == np.dtype(object) | ||
| np.testing.assert_array_equal(result, expected, strict=True) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_logical_xor_matches_unsupported_object_loop(self, operands, reverse): | ||
| quad, objects = operands | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| with pytest.raises(AttributeError): | ||
| np.logical_xor(left.astype(object), right.astype(object)) | ||
| with pytest.raises(AttributeError): | ||
| np.logical_xor(left, right) | ||
|
Comment on lines
+6570
to
+6573
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. AttributeError?? Is this test encoding a bug in numpy or something? |
||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_broadcasting_and_masked_out_use_object_loop(self, backend, reverse): | ||
| quad = np.array([[1], [2]], dtype=QuadPrecDType(backend=backend)) | ||
| objects = np.array([[3, 4, 5]], dtype=object) | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
| where = np.array([[True, False, True], [False, True, False]]) | ||
| out = np.full((2, 3), "unchanged", dtype=object) | ||
| expected = out.copy() | ||
|
|
||
| np.add(left, right, out=out, where=where) | ||
| np.add(left.astype(object), right.astype(object), out=expected, where=where) | ||
|
|
||
| np.testing.assert_array_equal(out, expected, strict=True) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_divmod_without_object_loop_still_raises(self, operands, reverse): | ||
|
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. why not add a divmod object loop? |
||
| quad, objects = operands | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| with pytest.raises(TypeError): | ||
| np.divmod(left, right) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize("reverse", [False, True]) | ||
| def test_matmul_uses_object_loop(self, backend, reverse): | ||
| quad = np.array([[1, 2], [3, 4]], dtype=QuadPrecDType(backend=backend)) | ||
| objects = np.array([[5, 6], [7, 8]], dtype=object) | ||
| left, right = (objects, quad) if reverse else (quad, objects) | ||
|
|
||
| result = np.matmul(left, right) | ||
| expected = np.matmul(left.astype(object), right.astype(object)) | ||
|
|
||
| assert result.dtype == np.dtype(object) | ||
| np.testing.assert_array_equal(result, expected, strict=True) | ||
|
|
||
| @pytest.mark.parametrize("backend", ["sleef", "longdouble"], indirect=True) | ||
| @pytest.mark.parametrize( | ||
| "other", | ||
| [ | ||
| np.array([3 + 1j, 4 + 2j], dtype=np.complex128), | ||
| np.array(["3", "4"], dtype="U1"), | ||
| np.array([b"3", b"4"], dtype="S1"), | ||
| ], | ||
| ) | ||
| def test_unsupported_common_dtypes_still_raise(self, operands, other): | ||
| quad, _ = operands | ||
|
|
||
| with pytest.raises(np.exceptions.DTypePromotionError): | ||
| np.result_type(quad, other) | ||
| with pytest.raises(TypeError): | ||
| np.add(quad, other) | ||
|
|
||
| def test_builtin_object_dispatch_is_unchanged(self): | ||
| left = np.array([1, 2], dtype=object) | ||
| right = np.array([3, 4], dtype=object) | ||
|
|
||
| result = np.add(left, right) | ||
|
|
||
| assert result.dtype == np.dtype(object) | ||
| np.testing.assert_array_equal(result, np.array([4, 6], dtype=object), strict=True) | ||
|
|
||
|
|
||
| def test_sleef_purecfma_symbols(): | ||
| """Test that SLEEF PURECFMA symbols are present in the compiled module. | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a one-line comment that this is needed to match numpy's semantics, which use bools for the comparison loops but object dtype for the
logical_functions.