Repository: python-graphblas/python-graphblas

Issue:
`first` and `second` maybe use the first or second argument determine result dtype
If one is using `first` or `second` binary operation, then the dtype of the other is probably irrelevant.  For example, here is a dtype-agnostic triangle count:
```python
# Burkhardt method: num_triangles = sum(sum(A @ A) * A) / 6
val1 = A.mxm(A, gb.semiring.plus_pair[gb.dtypes.UINT64]).new()
val2 = A.ewise_mult(val1, gb.binary.second).new()
# val2 = val1.ewise_mult(A, gb.binary.first).new()  # equivalent
num_triangles = val2.reduce_scalar().new().value // 6
```
If the dtype of `A` is a float, then `val2` will become a float.  I think it *may* be more natural to remain an int (although I'm not fully convinced of this).

A couple other things to note about this example.  I specify the dtype of `gb.semiring.plus_pair[gb.dtypes.UINT64]` so it works on booleans.  Recall that `pair` returns `1` in the same dtype, so `pair(x, y)` returns `True` for booleans.  `plus_pair` doesn't work, because `plus` doesn't work on booleans.  I think I'm fine needing to call this out explicitly, because the logic for why it's necessary makes sense.

One final point.  `val2` could/should instead be calculated as:
```python
val2 = val1.dup(mask=A.S)
```
which I think is clearer than using `first` or `second`.

Anyway.  `first` and `second` may be special enough (especially given their names) to have different rules for determining output dtype.

Discussion:
coveralls: ## Pull Request Test Coverage Report for [Build 2186395616](https://coveralls.io/builds/48363220)

* **27** of **27**   **(100.0%)**  changed or added relevant lines in **2** files are covered.
* **17** unchanged lines in **3** files lost coverage.
* Overall coverage remained the same at **99.714%**

---


|  Files with Coverage Reduction | New Missed Lines | % |
| :-----|--------------|--: |
| [graphblas/matrix.py](https://coveralls.io/builds/48363220/source?filename=graphblas%2Fmatrix.py#L809) | 4 | 99.56% |
| [graphblas/vector.py](https://coveralls.io/builds/48363220/source?filename=graphblas%2Fvector.py#L721) | 5 | 99.16% |
| [graphblas/__init__.py](https://coveralls.io/builds/48363220/source?filename=graphblas%2F__init__.py#L191) | 8 | 93.84% |
<!-- | **Total:** | **17** |  | -->

|  Totals | [![Coverage Status](https://coveralls.io/builds/48363220/badge)](https://coveralls.io/builds/48363220) |
| :-- | --: |
| Change from base [Build 2186401366](https://coveralls.io/builds/48363644): |  0% |
| Covered Lines: | 15165 |
| Relevant Lines: | 15193 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/graphblas/_ss/matrix.py b/graphblas/_ss/matrix.py
index c44a5c2..b294112 100644
--- a/graphblas/_ss/matrix.py
+++ b/graphblas/_ss/matrix.py
@@ -828,7 +828,7 @@ class ss:
                     - values : ndarray(ndim=2, shape=(nrows, ncols), order="F")
                 - ``raw=True``
                     - values : ndarray(ndim=1, size=nrows * ncols)
-                        - Stored row-oriented
+                        - Stored column-oriented
                     - nrows : int
                     - ncols : int
             - "coor" format
diff --git a/graphblas/_ss/vector.py b/graphblas/_ss/vector.py
index e17201c..ddba6ce 100644
--- a/graphblas/_ss/vector.py
+++ b/graphblas/_ss/vector.py
@@ -433,13 +433,6 @@ class ss:
                 - nvals : int
                     - The number of True elements in the bitmap
                 - size : int, only present if raw == True
-            - "bitmap" format
-                - bitmap : ndarray(dtype=bool8, size=size)
-                - values : ndarray(size=size)
-                    - Elements where bitmap is False are undefined
-                - nvals : int
-                    - The number of True elements in the bitmap
-                - size : int, only present if raw == True
             - "full" format
                 - values : ndarray(size=size)
                 - size : int, only present if raw == True or is_iso == True
diff --git a/graphblas/operator.py b/graphblas/operator.py
index 1e589e9..b9dbc8c 100644
--- a/graphblas/operator.py
+++ b/graphblas/operator.py
@@ -434,6 +434,7 @@ def _deserialize_parameterized(parameterized_op, args, kwargs):
 class ParameterizedUdf:
     __slots__ = "name", "__call__", "_anonymous", "__weakref__"
     is_positional = False
+    _custom_dtype = None
 
     def __init__(self, name, anonymous):
         self.name = name
@@ -849,6 +850,7 @@ def _one(x):
 
 class UnaryOp(OpBase):
     __slots__ = "orig_func", "is_positional", "_is_udt", "_numba_func"
+    _custom_dtype = None
     _module = unary
     _modname = "unary"
     _typed_class = TypedBuiltinUnaryOp
@@ -1113,6 +1115,7 @@ class SelectOp(OpBase):
     _module = select
     _modname = "select"
     _is_udt = False
+    _custom_dtype = None
     _typed_class = TypedBuiltinSelectOp
     _parse_config = {
         "trim_from_front": 4,
@@ -1199,6 +1202,24 @@ def _pair(x, y):
     return 1  # pragma: no cover
 
 
+def _first_dtype(op, dtype, dtype2):
+    if dtype._is_udt:
+        return op._compile_udt(dtype, dtype2)
+    else:
+        return op[dtype]
+
+
+def _second_dtype(op, dtype, dtype2):
+    if dtype2._is_udt:
+        return op._compile_udt(dtype, dtype2)
+    else:
+        return op[dtype2]
+
+
+def _pair_dtype(op, dtype, dtype2):
+    return op[INT64]
+
+
 def _get_udt_wrapper(numba_func, return_type, dtype, dtype2=None):
     ztype = INT8 if return_type == BOOL else return_type
     xtype = INT8 if dtype == BOOL else dtype
@@ -1273,6 +1294,7 @@ class BinaryOp(OpBase):
         "is_positional",
         "_is_udt",
         "_numba_func",
+        "_custom_dtype",
     )
     _module = binary
     _modname = "binary"
@@ -1747,6 +1769,10 @@ class BinaryOp(OpBase):
         binary.eq._udt_ops = {}
         binary.ne._udt_types = {}
         binary.ne._udt_ops = {}
+        # Set custom dtype handling
+        binary.first._custom_dtype = _first_dtype
+        binary.second._custom_dtype = _second_dtype
+        binary.pair._custom_dtype = _pair_dtype
         cls._initialized = True
 
     def __init__(
@@ -1767,6 +1793,7 @@ class BinaryOp(OpBase):
         self._numba_func = numba_func
         self._is_udt = is_udt
         self.is_positional = is_positional
+        self._custom_dtype = None
         if is_udt:
             self._udt_types = {}  # {(dtype, dtype): DataType}
             self._udt_ops = {}  # {(dtype, dtype): TypedUserBinaryOp}
@@ -1796,6 +1823,7 @@ class Monoid(OpBase):
     __slots__ = "_binaryop", "_identity"
     is_commutative = True
     is_positional = False
+    _custom_dtype = None
     _module = monoid
     _modname = "monoid"
     _typed_class = TypedBuiltinMonoid
@@ -2338,6 +2366,10 @@ class Semiring(OpBase):
     def _is_udt(self):
         return self._binaryop is not None and self._binaryop._is_udt
 
+    @property
+    def _custom_dtype(self):
+        return self.binaryop._custom_dtype
+
     commutes_to = TypedBuiltinSemiring.commutes_to
     is_commutative = TypedBuiltinSemiring.is_commutative
     __call__ = TypedBuiltinSemiring.__call__
@@ -2345,20 +2377,28 @@ class Semiring(OpBase):
 
 def get_typed_op(op, dtype, dtype2=None, *, is_left_scalar=False, is_right_scalar=False, kind=None):
     if isinstance(op, OpBase):
+        # UDTs always get compiled
         if op._is_udt:
             return op._compile_udt(dtype, dtype2)
-        if dtype2 is not None:
-            try:
-                dtype = unify(
-                    dtype, dtype2, is_left_scalar=is_left_scalar, is_right_scalar=is_right_scalar
-                )
-            except (TypeError, AttributeError):
-                if op.is_positional:
-                    return op[UINT64]
-                if op._udt_types is None:
-                    raise
+        # Single dtype is simple lookup
+        elif dtype2 is None:
+            return op[dtype]
+        # Handle special cases such as first and second (may have UDTs)
+        elif op._custom_dtype is not None:
+            return op._custom_dtype(op, dtype, dtype2)
+        # Generic case: try to unify the two dtypes
+        try:
+            return op[
+                unify(dtype, dtype2, is_left_scalar=is_left_scalar, is_right_scalar=is_right_scalar)
+            ]
+        except (TypeError, AttributeError):
+            # Failure to unify implies a dtype is UDT; some builtin operators can handle UDTs
+            if op.is_positional:
+                return op[UINT64]
+            elif op._udt_types is None:
+                raise
+            else:
                 return op._compile_udt(dtype, dtype2)
-        return op[dtype]
     elif isinstance(op, ParameterizedUdf):
         op = op()  # Use default parameters of parameterized UDFs
         return get_typed_op(


Test patch:
diff --git a/graphblas/tests/test_op.py b/graphblas/tests/test_op.py
index 639b8dd..431aaf2 100644
--- a/graphblas/tests/test_op.py
+++ b/graphblas/tests/test_op.py
@@ -1158,8 +1158,12 @@ def test_udt():
 
     assert binary.second[udt].type is udt
     assert binary.second[udt].type2 is udt
-    assert binary.second[udt, dtypes.INT8].type is udt
+    assert binary.second[udt, dtypes.INT8].type is dtypes.INT8  # ignore first arg
     assert binary.second[udt, dtypes.INT8].type2 is dtypes.INT8
+    assert semiring.any_second[udt, dtypes.INT8].type is dtypes.INT8  # ignore first arg
+    assert semiring.any_second[udt, dtypes.INT8].type2 is dtypes.INT8
+    assert binary.first[udt, dtypes.INT8].type is udt
+    assert binary.first[udt, dtypes.INT8].type2 is dtypes.INT8  # include second arg!
     assert monoid.any[udt].type2 is udt