BUG: Preserve object common-dtype promotion in Quad ufuncs - #118
BUG: Preserve object common-dtype promotion in Quad ufuncs#118SwayamInSync wants to merge 2 commits into
Conversation
|
This is small addition but important to ensure we do not override NumPy's default |
ngoldbaum
left a comment
ngoldbaum
left a comment
There was a problem hiding this comment.
Sorry for taking so long! Just one comment about the promoters below and a request for a comment. Also maybe take a look if the promoters are overly-broad right now elsewhere in the implementation (try to avoid promoters that match for all dtypes).
|
|
||
|
|
||
| static bool | ||
| comparison_object_output_is_bool(PyUFuncObject *ufunc) |
There was a problem hiding this comment.
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.
| @@ -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); | |||
There was a problem hiding this comment.
should this one and the one on line 373 use PyArray_ObjectDType as the result and "other" operand dtype? For changing the result dtype, this works just fine (for example):
>>> import numpy as np
>>> a = np.array(4, dtype='O')
>>> b = np.array(4.5)
>>> result = np.array(7, dtype='O')
>>> np.equal(a, b, out=result)
array(False, dtype=object)
>>> result
array(False, dtype=object)
The way you have it written here with exactly Bool as the output doesn't match that.
Also use PyArray_ObjectDtype instead of PyArrayDescr_Type, the latter fires for all dtypes so even if that doesn't lead to a bug and cause weirdness in unrelated dtypes, it's also a performance hit for all ufunc dispatch for all other dtypes.
There was a problem hiding this comment.
Addressed and the 2nd point is also validated #118 (comment)
| with pytest.raises(AttributeError): | ||
| np.logical_xor(left.astype(object), right.astype(object)) | ||
| with pytest.raises(AttributeError): | ||
| np.logical_xor(left, right) |
There was a problem hiding this comment.
AttributeError?? Is this test encoding a bug in numpy or something?
There was a problem hiding this comment.
This is from NumPy. np.logical_xor does register an object loop (OO->O), but NumPy's object loop calls a method named after the ufunc on each element, so it looks for int.logical_xor and raises AttributeError.
An object that defines logical_xor works fine. np.logical_xor(obj_arr, obj_arr) raises the same AttributeError without any quad involved.
|
|
||
| @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): |
There was a problem hiding this comment.
why not add a divmod object loop?
There was a problem hiding this comment.
Because NumPy has no object loop for it, np.divmod.types contains no O entries at all, unlike floor_divide and remainder which both have OO->O.
|
I addressed the reviews No contamination of unrelated dtypes. Every one of quaddtype's promoter patterns pins concrete |
2 participants
Closes #114
The promoters now preserve NumPy's ObjectDType precedence. When a Quad ufunc receives an object-array input, it selects NumPy's existing object loop rather than forcing the object operand to Quad.
This applies in both operand orders and covers:
addandmultiplymatmulout=operationsOperations without an applicable object loop continue to raise normally. Unsupported Quad/complex and Quad/string promotion is unchanged, and importing quaddtype does not affect built-in-only NumPy dispatch.
The change also makes matmul promoter registration failures propagate instead of being silently cleared.