From c0c4744bc24147751f88ce553f625c781547b4d9 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 5 Aug 2026 01:34:35 -0500 Subject: [PATCH 1/6] Extend the monoid guard to built-ins that carry a UDT fallback func The guard added with the monoid-from-built-in rejection keyed on "constructed with neither a Python func nor a numba func", which is true for most built-ins but not all of them: binary.any, first, pair, and second carry a Python orig_func as their UDT fallback while their typed ops are still TypedBuiltinBinaryOp. All four sailed past the guard and reproduced exactly what it exists to prevent: an opaque AttributeError partway through registration, with binary.._monoid left pointing at the half-built monoid (binary.any.monoid is monoid.any flipped from True to False after the rejected call). binary.any is even named in the guard's own rationale as an op whose identity SuiteSparse ignores. Key the probe on the typed ops as well: any TypedBuiltinBinaryOp among them marks the op built-in. The two signals agree everywhere else, including under mapnumpy=True, where the mapped numpy binary ops hold the built-in's typed ops and no funcs, and are flagged by both tests alike; the numpy monoids themselves are mapped without passing through Monoid._build, so their registration is unaffected (verified under both mapnumpy settings). A UDF's typed ops are never TypedBuiltinBinaryOp, so nothing legitimate is newly rejected. --- graphblas/core/operator/monoid.py | 19 +++++++++++++++---- graphblas/tests/test_op.py | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index bcf06d323..d5041fb2c 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -32,11 +32,22 @@ def _is_builtin_binaryop(binaryop): """True if ``binaryop`` wraps built-in GraphBLAS operators rather than a UDF. - Built-ins are the only binary ops constructed with neither a Python func nor - a numba func. That covers ``binary.numpy.float_power`` too, which is - assembled out of ``binary.pow``'s typed ops. + Most built-ins are constructed with neither a Python func nor a numba func. + That covers ``binary.numpy.float_power`` too, which is assembled out of + ``binary.pow``'s typed ops. A few built-ins (``any``, ``first``, ``pair``, + ``second``) carry a Python ``orig_func`` as their UDT fallback, so the func + test alone misses them; their typed ops are still ``TypedBuiltinBinaryOp``, + which is the authoritative signal. A UDF's typed ops are never + ``TypedBuiltinBinaryOp``. Under ``mapnumpy=True`` the mapped numpy binary + ops hold the built-in's typed ops and no funcs, so both tests agree they + are built-in; the numpy monoids themselves are mapped without passing + through ``Monoid._build``. """ - return binaryop.orig_func is None and binaryop._numba_func is None + return ( + binaryop.orig_func is None + and binaryop._numba_func is None + or any(type(typed) is TypedBuiltinBinaryOp for typed in binaryop._typed_ops.values()) + ) def _scalar_identity(monoid_name, scalar_dtype): diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index c76c895df..40b93d06c 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -440,6 +440,24 @@ def test_monoid_rejects_builtin_binaryop(): assert not hasattr(gb.monoid, "_bad_builtin_monoid") +def test_monoid_rejects_builtin_binaryop_with_udt_fallback(): + """binary.any, first, pair, and second are built-ins that carry a Python + func as their UDT fallback, so a func-based probe alone misses them. They + must be rejected like the other built-ins: their typed ops have no _monoid + slot, and a failed registration used to leave binary.._monoid + pointing at the half-built monoid. + """ + for binop in [binary.any, binary.first, binary.pair, binary.second]: + with pytest.raises(TypeError, match="must be a user-defined BinaryOp"): + Monoid.register_anonymous(binop, 0) + # State is untouched: any's built-in monoid association survives, and the + # ops without a built-in monoid still have none. + assert binary.any.monoid is monoid.any + assert binary.first.monoid is None + assert binary.pair.monoid is None + assert binary.second.monoid is None + + @pytest.mark.skipif("not supports_udfs") @pytest.mark.slow def test_semiring_parameterized(): From f55172393ce2ef6a4ec60a4ebbf3104bef6fa8e1 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 27 Oct 2026 09:49:15 -0500 Subject: [PATCH 2/6] Give a timedelta64 field the same KeyError as a datetime64 one in min/max Behavior fix. monoid.min and monoid.max generate a UDT's identity field by field, and _scalar_identity picked the integer arm with np.issubdtype(dtype, np.integer). numpy files timedelta64 under np.integer, so a record with a timedelta64 field reached np.iinfo, which rejects it: rec = dtypes.register_anonymous(np.dtype([("n", np.int64), ("t", "m8[s]")])) monoid.min[rec] # before: ValueError: Invalid integer data type 'm'. rec in agg.min # before: the same ValueError, not False A datetime64 field already got "KeyError: monoid.min does not work with ...: field 't' has unsupported dtype datetime64[s]", and `in` answered False. Test the dtype's kind instead, so a timedelta64 field takes that path too: monoid.min[rec] # after: KeyError ... field 't' has unsupported dtype timedelta64[s] rec in agg.min # after: False Real identities for timedelta64 fields (its own max and min, with NaT in mind) are a separate enhancement. --- graphblas/core/operator/monoid.py | 4 +++- graphblas/tests/test_op.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index d5041fb2c..a1b44c210 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -64,7 +64,9 @@ def _scalar_identity(monoid_name, scalar_dtype): return scalar_dtype.type(1) if monoid_name not in {"min", "max"}: return None - if np.issubdtype(scalar_dtype, np.integer): + # Not ``np.issubdtype(scalar_dtype, np.integer)``: numpy files timedelta64 + # under ``np.integer``, and ``np.iinfo`` rejects it. + if scalar_dtype.kind in "iu": info = np.iinfo(scalar_dtype) return info.max if monoid_name == "min" else info.min if np.issubdtype(scalar_dtype, np.floating): diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index 40b93d06c..bcf307b8e 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -2474,6 +2474,25 @@ def test_udt_auto_monoid(): assert any_res in ((7, 8.0), (11, 12.0)) +@pytest.mark.skipif("not supports_udfs") +@pytest.mark.slow +def test_udt_auto_monoid_time_fields(): + """Monoid min and max generate no identity for a datetime64 or timedelta64 field. + + numpy files timedelta64 under ``np.integer``, so a timedelta64 field used to + reach ``np.iinfo`` and raise ``ValueError: Invalid integer data type 'm'``, + from ``udt in agg.min`` too, where a datetime64 field gets a KeyError. + """ + for code in ["M8[s]", "m8[s]"]: + udt = dtypes.register_anonymous(np.dtype([("tf_n", np.int64), ("tf_t", code)])) + for mon in [monoid.min, monoid.max]: + with pytest.raises(KeyError, match="field 'tf_t' has unsupported dtype"): + mon[udt] + assert udt not in mon + assert udt not in agg.min + assert udt not in agg.max + + @pytest.mark.skipif("not supports_udfs") @pytest.mark.parametrize("shape", [(4,), (3, 2)]) def test_udt_array_wrapper_stays_within_element(shape): From 4fa61c03f8772ea4c568b8166d3ac37ee60d5312 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 27 Oct 2026 09:49:50 -0500 Subject: [PATCH 3/6] Say why a UDT monoid fails when its op returns another type Behavior fix. Monoid._compile_udt handed a binary op whose return type differs from its input type straight to GrB_Monoid_new_UDT, which rejects it with a DomainMismatch that has no message, also from `in`: op = BinaryOp.register_anonymous(lambda x, y: x["a"] + y["a"], is_udt=True) m = Monoid.register_anonymous(op, 0) m[rec] # before: DomainMismatch (empty message) rec in m # before: the same DomainMismatch Check the compiled op's return type first and raise a TypeError, the error the neighbouring check (two different input types) raises: m[rec] # after: TypeError: monoid. does not work with rec: a # monoid's binary operator must return its input type, rec, not INT64 rec in m # after: False --- graphblas/core/operator/monoid.py | 7 +++++++ graphblas/tests/test_op.py | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index a1b44c210..3fbb69418 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -353,6 +353,13 @@ def _compile_udt(self, dtype, dtype2): from ..scalar import Scalar ret_type = binaryop.return_type + if ret_type != dtype: + # Checked here because GrB_Monoid_new_UDT reports it as a DomainMismatch + # with no message. + raise TypeError( + f"monoid.{self.name} does not work with {dtype}: a monoid's binary operator " + f"must return its input type, {dtype}, not {ret_type}" + ) identity_val = self._identity if identity_val is None and self.name in _BUILTIN_UDT_MONOIDS: # Auto-generate the identity for built-in monoids on UDTs. diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index bcf307b8e..b804ca161 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -1999,6 +1999,25 @@ def _add_udt(x, y): assert result.isequal(expected) +@pytest.mark.skipif("not supports_udfs") +@pytest.mark.slow +def test_udt_monoid_op_must_return_its_input_type(record_udt): + """A UDT monoid whose op returns another type fails up front, with a message. + + It used to reach ``GrB_Monoid_new_UDT``, which raised DomainMismatch with an + empty message, also from ``udt in monoid``. + """ + + def _sum_a(x, y): + return x["a"] + y["a"] # pragma: no cover (numba) + + sum_a = BinaryOp.register_anonymous(_sum_a, "test_sum_a_mon", is_udt=True) + sum_a_monoid = Monoid.register_anonymous(sum_a, 0) + with pytest.raises(TypeError, match="must return its input type, _BuiltinOpsRec, not INT64"): + sum_a_monoid[record_udt] + assert record_udt not in sum_a_monoid + + @pytest.mark.skipif("not supports_udfs") @pytest.mark.slow def test_udt_tuple_return_semiring(record_udt): From 28740050856a287c499542524cd15169f2078d26 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 27 Oct 2026 09:50:35 -0500 Subject: [PATCH 4/6] Keep is_idempotent through Monoid.register_new(lazy=True) Behavior fix. The lazy branch stored register_new's arguments for the delayed call but left is_idempotent out, so the monoid built on first access was never idempotent: Monoid.register_new("m", op, 0, is_idempotent=True, lazy=True) monoid.m.is_idempotent # before: False # after: True The eager branch already passed it. The test registers lazy_idem in the binary and monoid namespaces, so test_operator_types ignores it like the other names test_lazy_op makes. --- graphblas/core/operator/monoid.py | 7 ++++++- graphblas/tests/test_op.py | 3 +++ graphblas/tests/test_operator_types.py | 1 + 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index 3fbb69418..676da7e0f 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -459,7 +459,12 @@ def register_new(cls, name, binaryop, identity, *, is_idempotent=False, lazy=Fal if lazy: module._delayed[funcname] = ( cls.register_new, - {"name": name, "binaryop": binaryop, "identity": identity}, + { + "name": name, + "binaryop": binaryop, + "identity": identity, + "is_idempotent": is_idempotent, + }, ) elif type(binaryop) is ParameterizedBinaryOp: monoid = ParameterizedMonoid(name, binaryop, identity, is_idempotent=is_idempotent) diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index b804ca161..8a15803f7 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -1206,6 +1206,9 @@ def test_lazy_op(): Monoid.register_new("lazy2", binary.lazy, 0, lazy=True) assert isinstance(op.lazy2, Monoid) assert isinstance(monoid.lazy2, Monoid) + BinaryOp.register_new("lazy_idem", _isidem_pick_first, lazy=True) + Monoid.register_new("lazy_idem", "lazy_idem", 0, is_idempotent=True, lazy=True) + assert monoid.lazy_idem.is_idempotent Semiring.register_new("lazy", "lazy", "lazy", lazy=True) assert isinstance(semiring.lazy, Semiring) Semiring.register_new("lazy_lazy", monoid.lazy, binary.lazy, lazy=True) diff --git a/graphblas/tests/test_operator_types.py b/graphblas/tests/test_operator_types.py index 8c250bc25..9192b37ae 100644 --- a/graphblas/tests/test_operator_types.py +++ b/graphblas/tests/test_operator_types.py @@ -185,6 +185,7 @@ "plus_copysign", "lazy", "lazy2", + "lazy_idem", "lazy_lazy", # UDFs created during tests "is_positive", From d0260fdb835a81454ad18bf61d4f6e4a6d2e980e Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Tue, 27 Oct 2026 09:51:47 -0500 Subject: [PATCH 5/6] Generate a UDT identity for the built-in monoids only, not by name Behavior fix. Monoid._compile_udt generated a field-wise identity whenever the monoid had no identity and its name was plus, times, min or max. A user monoid gets its name from its BinaryOp, which gets it from the function, so a user op called `min` (or a monoid given name="plus") silently took the built-in's identity and gave wrong results. With an op that takes the field-wise max and identity None: op = BinaryOp.register_anonymous(fieldwise_max, "min", is_udt=True) v = [(1, 2), (5, -3)]; w = [(-1, -2), (-5, -3)] Monoid.register_anonymous(op, None) # name "min" before: identity (INT64_MAX, INT64_MAX); v.reduce -> (INT64_MAX, INT64_MAX) Monoid.register_anonymous(op, None, "plus") before: identity (0, 0); v.reduce -> (5, 2), w.reduce -> (0, 0) both, after: KeyError: monoid. does not work with ...: no identity value Generate the identity only for a built-in monoid, the ones with no _binaryop of their own. A user monoid with an identity is unaffected. --- graphblas/core/operator/monoid.py | 6 ++++-- graphblas/tests/test_op.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/graphblas/core/operator/monoid.py b/graphblas/core/operator/monoid.py index 676da7e0f..e6d2ce07e 100644 --- a/graphblas/core/operator/monoid.py +++ b/graphblas/core/operator/monoid.py @@ -361,8 +361,10 @@ def _compile_udt(self, dtype, dtype2): f"must return its input type, {dtype}, not {ret_type}" ) identity_val = self._identity - if identity_val is None and self.name in _BUILTIN_UDT_MONOIDS: - # Auto-generate the identity for built-in monoids on UDTs. + if identity_val is None and self._binaryop is None and self.name in _BUILTIN_UDT_MONOIDS: + # Auto-generate the identity for built-in monoids on UDTs. Only built-in + # monoids have no ``_binaryop``: a user monoid named like one (after its + # op's function, or by ``name=``) must not get that identity. identity_val = _udt_identity(self.name, ret_type) if identity_val is None: raise KeyError( diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index 8a15803f7..047df7d36 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -2515,6 +2515,37 @@ def test_udt_auto_monoid_time_fields(): assert udt not in agg.max +@pytest.mark.skipif("not supports_udfs") +@pytest.mark.slow +def test_udt_monoid_named_like_builtin_gets_no_identity(): + """Only the built-in monoids generate a UDT identity, not a user monoid of the same name. + + A user monoid named plus, times, min or max (after its op's function, or by + ``name=``) used to get that built-in's generated identity. The op here takes + the field-wise max, so the generated ``min`` identity, INT64_MAX in every + field, absorbed every value: the reduce below gave that instead of (5, 2). + """ + udt = dtypes.register_anonymous(np.dtype([("ni_a", np.int64), ("ni_b", np.int64)])) + v = Vector(udt, size=2) + v[0] = (1, 2) + v[1] = (5, -3) + + def _fieldwise_max(x, y): + return (max(x["ni_a"], y["ni_a"]), max(x["ni_b"], y["ni_b"])) # pragma: no cover (numba) + + fieldwise_max = BinaryOp.register_anonymous(_fieldwise_max, "min", is_udt=True) + named_by_op = Monoid.register_anonymous(fieldwise_max, None) + assert named_by_op.name == "min" + for mon in [named_by_op] + [ + Monoid.register_anonymous(fieldwise_max, None, name) for name in ["plus", "times", "max"] + ]: + with pytest.raises(KeyError, match="no identity value"): + mon[udt] + assert udt not in mon + lowest = np.iinfo(np.int64).min + assert v.reduce(Monoid.register_anonymous(fieldwise_max, (lowest, lowest))).new() == (5, 2) + + @pytest.mark.skipif("not supports_udfs") @pytest.mark.parametrize("shape", [(4,), (3, 2)]) def test_udt_array_wrapper_stays_within_element(shape): From d94407fc01f3d45a1a260ba81c1b605769beddb1 Mon Sep 17 00:00:00 2001 From: Erik Welch Date: Wed, 28 Oct 2026 00:22:43 -0500 Subject: [PATCH 6/6] Match the UDT monoid error by the record UDT's current name test_udt_monoid_op_must_return_its_input_type matched the error against "_BuiltinOpsRec", the name the record_udt fixture registers. But register_anonymous returns the one DataType for an np.dtype and gives it the newest name, and test_udt_op_pickle registers the same record dtype as "_PickleUdt". In random order, with a record_udt test, then test_udt_op_pickle, then this test, the message said "_PickleUdt" and the match failed (wave 2 CI: 13 jobs, e.g. 122634118659 and 122634118406, on both backends and with mapnumpy on or off). Match the fixture's name as it is when the error is raised. --- graphblas/tests/test_op.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py index 047df7d36..594b196a6 100644 --- a/graphblas/tests/test_op.py +++ b/graphblas/tests/test_op.py @@ -2016,7 +2016,9 @@ def _sum_a(x, y): sum_a = BinaryOp.register_anonymous(_sum_a, "test_sum_a_mon", is_udt=True) sum_a_monoid = Monoid.register_anonymous(sum_a, 0) - with pytest.raises(TypeError, match="must return its input type, _BuiltinOpsRec, not INT64"): + # Not "_BuiltinOpsRec": registering this record dtype again under another name, as + # test_udt_op_pickle does, renames the one DataType + with pytest.raises(TypeError, match=f"must return its input type, {record_udt}, not INT64"): sum_a_monoid[record_udt] assert record_udt not in sum_a_monoid