Skip to content

Commit 682cd04

Browse files
sebergseberg
committed
BUG,ENH: Fix generic scalar infinite recursion issues
This reorganizes the avoidance of infinite recursion to happen in the generic scalar fallback, rather than attempting to do so (badly) in the scalarmath, where it only applies to longdouble to begin with. This was also needed for the follow up to remove special handling of python scalar subclasses.
1 parent 735a477 commit 682cd04

4 files changed

Lines changed: 276 additions & 53 deletions

File tree

‎numpy/_core/src/multiarray/npy_static_data.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ intern_strings(void)
4141
INTERN_STRING(implementation, "_implementation");
4242
INTERN_STRING(axis1, "axis1");
4343
INTERN_STRING(axis2, "axis2");
44+
INTERN_STRING(item, "item");
4445
INTERN_STRING(like, "like");
4546
INTERN_STRING(numpy, "numpy");
4647
INTERN_STRING(where, "where");

‎numpy/_core/src/multiarray/npy_static_data.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ typedef struct npy_interned_str_struct {
2323
PyObject *implementation;
2424
PyObject *axis1;
2525
PyObject *axis2;
26+
PyObject *item;
2627
PyObject *like;
2728
PyObject *numpy;
2829
PyObject *where;

‎numpy/_core/src/multiarray/scalartypes.c.src‎

Lines changed: 270 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include "ctors.h"
2222
#include "dtypemeta.h"
2323
#include "usertypes.h"
24+
#include "number.h"
2425
#include "numpyos.h"
2526
#include "can_cast_table.h"
2627
#include "common.h"
@@ -120,19 +121,6 @@ gentype_free(PyObject *v)
120121
}
121122

122123

123-
static PyObject *
124-
gentype_power(PyObject *m1, PyObject *m2, PyObject *modulo)
125-
{
126-
if (modulo != Py_None) {
127-
/* modular exponentiation is not implemented (gh-8804) */
128-
Py_INCREF(Py_NotImplemented);
129-
return Py_NotImplemented;
130-
}
131-
132-
BINOP_GIVE_UP_IF_NEEDED(m1, m2, nb_power, gentype_power);
133-
return PyArray_Type.tp_as_number->nb_power(m1, m2, Py_None);
134-
}
135-
136124
static PyObject *
137125
gentype_generic_method(PyObject *self, PyObject *args, PyObject *kwds,
138126
char *str)
@@ -164,33 +152,194 @@ gentype_generic_method(PyObject *self, PyObject *args, PyObject *kwds,
164152
}
165153
}
166154

167-
static PyObject *
168-
gentype_add(PyObject *m1, PyObject* m2)
169-
{
170-
/* special case str.__radd__, which should not call array_add */
171-
if (PyBytes_Check(m1) || PyUnicode_Check(m1)) {
172-
Py_INCREF(Py_NotImplemented);
173-
return Py_NotImplemented;
155+
156+
/*
157+
* Helper function to deal with binary operator deferral. Must be passed a
158+
* valid self (a generic scalar) and an other item.
159+
* May fill self_item and/or other_arr (but not both) with non-NULL values.
160+
*
161+
* Why this dance? When the other object is a exactly Python scalar something
162+
* awkward happens historically in NumPy.
163+
* NumPy doesn't define a result, but the ufunc would cast to `astype(object)`
164+
* which is the same as `scalar.item()`. And that operation converts e.g.
165+
* float32 or float64 to Python floats.
166+
* It then retries. And because it is a builtin type now the operation may
167+
* succeed.
168+
*
169+
* This retrying pass only makes sense if the other object is a Python
170+
* scalar (otherwise we fill in `other_arr` which can be used to call the
171+
* ufunc).
172+
* Additionally, if `self.item()` has the same type as `self` we would end up
173+
* in an infinite recursion.
174+
*
175+
* So the result of this function means the following:
176+
* - < 0 error return.
177+
* - self_op is filled in: Retry the Python operator.
178+
* - other_op is filled in: Use the array operator (goes into ufuncs)
179+
* (This may be the original generic if it is one.)
180+
* - neither is filled in: Return NotImplemented.
181+
*
182+
* It is not possible for both to be filled. If `other` is also a generics,
183+
* it is returned.
184+
*/
185+
static inline int
186+
find_binary_operation_path(
187+
PyObject *self, PyObject *other, PyObject **self_op, PyObject **other_op)
188+
{
189+
*other_op = NULL;
190+
*self_op = NULL;
191+
192+
if (PyArray_IsScalar(other, Generic) ||
193+
PyLong_CheckExact(other) ||
194+
PyFloat_CheckExact(other) ||
195+
PyComplex_CheckExact(other) ||
196+
PyBool_Check(other)) {
197+
/*
198+
* The other operand is ready for the operation already. Must pass on
199+
* on float/long/complex mainly for weak promotion (NEP 50).
200+
*/
201+
Py_INCREF(other);
202+
*other_op = other;
203+
return 0;
174204
}
175-
BINOP_GIVE_UP_IF_NEEDED(m1, m2, nb_add, gentype_add);
176-
return PyArray_Type.tp_as_number->nb_add(m1, m2);
205+
206+
/*
207+
* Now check `other`. We want to know whether it is an object scalar
208+
* and the easiest way is by converting to an array here.
209+
*/
210+
int was_scalar;
211+
PyArrayObject *arr = (PyArrayObject *)PyArray_FromAny_int(
212+
other, NULL, NULL, 0, 0, 0, NULL, &was_scalar);
213+
if (arr == NULL) {
214+
return -1;
215+
}
216+
217+
if (!was_scalar || PyArray_DESCR(arr)->type_num != NPY_OBJECT) {
218+
/* The array is OK for usage and we can simply forward it
219+
*
220+
* NOTE: Future NumPy may need to distinguish scalars here, one option
221+
* could be marking the array.
222+
*/
223+
*other_op = (PyObject *)arr;
224+
return 0;
225+
}
226+
Py_DECREF(arr);
227+
228+
/*
229+
* If we are here, we need to operate on Python scalars. In general
230+
* that would just fails since NumPy doesn't know the other object!
231+
*
232+
* However, NumPy (historically) often makes this work magically because
233+
* it object ufuncs end up casting to object with `.item()` and that may
234+
* returns Python type often (e.g. float for float32, float64)!
235+
* Retrying then succeeds. So if (and only if) `self.item()` returns a new
236+
* type, we can safely attempt the operation (again) with that.
237+
*/
238+
PyObject *self_item = PyObject_CallMethodNoArgs(self, npy_interned_str.item);
239+
if (self_item == NULL) {
240+
return -1;
241+
}
242+
if (Py_TYPE(self_item) != Py_TYPE(self)) {
243+
/* self_item can be used to retry the operation */
244+
*self_op = self_item;
245+
return 0;
246+
}
247+
/* The operation can't work and we will return NotImplemented */
248+
return 0;
177249
}
178250

251+
252+
/*
253+
* These are defined below as they require special handling, we still define
254+
* a _gen version here. `power` is special as it has three arguments.
255+
*/
256+
static PyObject *
257+
gentype_add(PyObject *m1, PyObject *m2);
258+
259+
static PyObject *
260+
gentype_multiply(PyObject *m1, PyObject *m2);
261+
262+
179263
/**begin repeat
180264
*
181-
* #name = subtract, remainder, divmod, lshift, rshift,
182-
* and, xor, or, floor_divide, true_divide#
265+
* #name = add, multiply, subtract, remainder, divmod,
266+
* lshift, rshift, and, xor, or, floor_divide, true_divide#
267+
* #ufunc = add, multiply, subtract, remainder, divmod,
268+
* left_shift, right_shift, bitwise_and, bitwise_xor, bitwise_or,
269+
* floor_divide, true_divide#
270+
* #func = Add, Multiply, Subtract, Remainder, Divmod,
271+
* Lshift, Rshift, And, Xor, Or, FloorDivide, TrueDivide#
272+
* #suff = _gen, _gen,,,,,,,,,,#
183273
*/
274+
/* NOTE: We suffix the name for functions requiring special handling first. */
184275
static PyObject *
185-
gentype_@name@(PyObject *m1, PyObject *m2)
276+
gentype_@name@@suff@(PyObject *m1, PyObject *m2)
186277
{
187278
BINOP_GIVE_UP_IF_NEEDED(m1, m2, nb_@name@, gentype_@name@);
188-
return PyArray_Type.tp_as_number->nb_@name@(m1, m2);
279+
280+
PyObject *self = NULL;
281+
PyObject *other = NULL;
282+
PyObject *self_op, *other_op;
283+
284+
if (!PyArray_IsScalar(m2, Generic)) {
285+
self = m1;
286+
other = m2;
287+
}
288+
else {
289+
self = m2;
290+
other = m1;
291+
}
292+
if (find_binary_operation_path(self, other, &self_op, &other_op) < 0) {
293+
return NULL;
294+
}
295+
if (self_op != NULL) {
296+
PyObject *res;
297+
if (self == m1) {
298+
res = PyNumber_@func@(self_op, m2);
299+
}
300+
else {
301+
res = PyNumber_@func@(m1, self_op);
302+
}
303+
Py_DECREF(self_op);
304+
return res;
305+
}
306+
else if (other_op != NULL) {
307+
/* Call the corresponding ufunc (with the array) */
308+
PyObject *res;
309+
if (self == m1) {
310+
res = PyArray_GenericBinaryFunction(m1, other_op, n_ops.@ufunc@);
311+
}
312+
else {
313+
res = PyArray_GenericBinaryFunction(other_op, m2, n_ops.@ufunc@);
314+
}
315+
Py_DECREF(other_op);
316+
return res;
317+
}
318+
else {
319+
assert(other_op == NULL);
320+
Py_RETURN_NOTIMPLEMENTED;
321+
}
189322
}
190323

191324
/**end repeat**/
192325

193-
/* Get a nested slot, or NULL if absent */
326+
/*
327+
* The following operators use the above, but require specialization.
328+
*/
329+
330+
static PyObject *
331+
gentype_add(PyObject *m1, PyObject *m2)
332+
{
333+
/* special case str.__radd__, which should not call array_add */
334+
if (PyBytes_Check(m1) || PyUnicode_Check(m1)) {
335+
Py_INCREF(Py_NotImplemented);
336+
return Py_NotImplemented;
337+
}
338+
339+
return gentype_add_gen(m1, m2);
340+
}
341+
342+
/* Get a nested slot, or NULL if absent (for multiply implementation) */
194343
#define GET_NESTED_SLOT(type, group, slot) \
195344
((type)->group == NULL ? NULL : (type)->group->slot)
196345

@@ -219,11 +368,75 @@ gentype_multiply(PyObject *m1, PyObject *m2)
219368
Py_INCREF(Py_NotImplemented);
220369
return Py_NotImplemented;
221370
}
222-
/* All normal cases are handled by PyArray's multiply */
223-
BINOP_GIVE_UP_IF_NEEDED(m1, m2, nb_multiply, gentype_multiply);
224-
return PyArray_Type.tp_as_number->nb_multiply(m1, m2);
371+
372+
return gentype_multiply_gen(m1, m2);
225373
}
226374

375+
376+
/*
377+
* NOTE: The three argument nature of power requires code duplication here.
378+
*/
379+
static PyObject *
380+
gentype_power(PyObject *m1, PyObject *m2, PyObject *modulo)
381+
{
382+
if (modulo != Py_None) {
383+
/* modular exponentiation is not implemented (gh-8804) */
384+
Py_INCREF(Py_NotImplemented);
385+
return Py_NotImplemented;
386+
}
387+
388+
BINOP_GIVE_UP_IF_NEEDED(m1, m2, nb_power, gentype_power);
389+
390+
PyObject *self = NULL;
391+
PyObject *other = NULL;
392+
PyObject *self_op, *other_op;
393+
394+
if (!PyArray_IsScalar(m2, Generic)) {
395+
self = m1;
396+
other = m2;
397+
}
398+
else {
399+
self = m2;
400+
other = m1;
401+
}
402+
if (find_binary_operation_path(self, other, &self_op, &other_op) < 0) {
403+
return NULL;
404+
}
405+
if (self_op != NULL) {
406+
PyObject *res;
407+
if (self == m1) {
408+
res = PyNumber_Power(self_op, m2, Py_None);
409+
}
410+
else {
411+
res = PyNumber_Power(m1, self_op, Py_None);
412+
}
413+
Py_DECREF(self_op);
414+
return res;
415+
}
416+
else if (other_op != NULL) {
417+
/* Call the corresponding ufunc (with the array)
418+
* NOTE: As of NumPy 2.0 there are inconsistencies in array_power
419+
* calling it would fail a (niche) test because an array is
420+
* returned in one of the fast-paths.
421+
* (once NumPy propagates 0-D arrays, this is irrelevant)
422+
*/
423+
PyObject *res;
424+
if (self == m1) {
425+
res = PyArray_GenericBinaryFunction(m1, other_op, n_ops.power);
426+
}
427+
else {
428+
res = PyArray_GenericBinaryFunction(other_op, m2, n_ops.power);
429+
}
430+
Py_DECREF(other_op);
431+
return res;
432+
}
433+
else {
434+
assert(other_op == NULL);
435+
Py_RETURN_NOTIMPLEMENTED;
436+
}
437+
}
438+
439+
227440
/**begin repeat
228441
* #TYPE = BYTE, UBYTE, SHORT, USHORT, INT, UINT,
229442
* LONG, ULONG, LONGLONG, ULONGLONG#
@@ -1265,8 +1478,6 @@ static PyNumberMethods gentype_as_number = {
12651478
static PyObject *
12661479
gentype_richcompare(PyObject *self, PyObject *other, int cmp_op)
12671480
{
1268-
PyObject *arr, *ret;
1269-
12701481
/*
12711482
* If the other object is None, False is always right. This avoids
12721483
* the array None comparison, at least until deprecation it is fixed.
@@ -1287,17 +1498,35 @@ gentype_richcompare(PyObject *self, PyObject *other, int cmp_op)
12871498

12881499
RICHCMP_GIVE_UP_IF_NEEDED(self, other);
12891500

1290-
arr = PyArray_FromScalar(self, NULL);
1291-
if (arr == NULL) {
1501+
PyObject *self_op;
1502+
PyObject *other_op;
1503+
if (find_binary_operation_path(self, other, &self_op, &other_op) < 0) {
12921504
return NULL;
12931505
}
1294-
/*
1295-
* Call via PyObject_RichCompare to ensure that other.__eq__
1296-
* has a chance to run when necessary
1297-
*/
1298-
ret = PyObject_RichCompare(arr, other, cmp_op);
1299-
Py_DECREF(arr);
1300-
return ret;
1506+
1507+
/* We can always just call RichCompare again */
1508+
if (other_op != NULL) {
1509+
/* If we use richcompare again, need to ensure that one op is array */
1510+
self_op = PyArray_FromScalar(self, NULL);
1511+
if (self_op == NULL) {
1512+
Py_DECREF(other_op);
1513+
return NULL;
1514+
}
1515+
PyObject *res = PyObject_RichCompare(self_op, other_op, cmp_op);
1516+
Py_DECREF(self_op);
1517+
Py_DECREF(other_op);
1518+
return res;
1519+
}
1520+
else if (self_op != NULL) {
1521+
/* Try again, since other is an object scalar and this one mutated */
1522+
PyObject *res = PyObject_RichCompare(self_op, other, cmp_op);
1523+
Py_DECREF(self_op);
1524+
return res;
1525+
}
1526+
else {
1527+
/* Comparison with arbitrary objects cannot be defined. */
1528+
Py_RETURN_NOTIMPLEMENTED;
1529+
}
13011530
}
13021531

13031532
static PyObject *

0 commit comments

Comments
 (0)