Open eriknw opened 1 year ago
I hadn't thought of what happens when GrBONEB or GxBPAIR is used as a comparator. I think the current behavior with GrB_ONEB_BOOL (aka GxB_PAIR_BOOL) is just an accident. This is because !(a < b) is not the same as (a >= b) for this ZERO_BOOL operator. I kind of assume the comparator "works", and that the values being sorted have a global ordering.
In other words, if op(x,y) is the comparator, I could assume that if x is not equal to y, that op(x,y) should be the same as !op(y,x). I don't use that condition, but I think the sort would only be defined if that condition is always true for the given op and the data types that are being sorted.
That's why I don't have a sort for complex values, for example. It's possible to define a total ordering of complex numbers, like MATLAB does, but that's not conventional and I think it's a mistake for MATLAB to do that.
In any case, using GxB_PAIR_BOOL doesn't trigger a "built-in" sort, but treats the operator as if it were user-defined anyway, and calls the "generic" method to sort a matrix or vector where the function pointer from the op is used.
So it would be just as fast if you were to create your own user-defined ZERO_BOOL operator. And just as undefined as the current behavior with GrB_ONEB_BOOL.
I do like the idea of fleshing out the table of all-possible binary boolean operators (adding ZERO, NOR, NOT(y), NOT(x), and NAND). But adding them (and ZERO in particular) wouldn't help with the sort. The sort would still be relying on undefined behavior.
For the "compactify" behavior, it might make more sense to have this as an entirely different function, or to have a comparator function that is a GrB_IndexBinaryOp. All the op would do is to say that the comparison of A(i1,j1) and A(i2,j2) is the test "j1 < j2". That would give you "compactify" by column. Now all we need are GrB_IndexBinaryOps.
Thanks for the reply.
Heh, yeah, I thought this might be abusing GxB_sort. Good to know this is undefined behavior.
I really like your idea of using a IndexBinaryOp to do what I want. +1 for IndexBinaryOp.
And I agree that you don't want to sort complex with e.g. LT. Python doesn't allow this either. I think it's best to require a user-defined function if users want to sort complex.
I should mention the undefined behavior in the user guide. I don't plan on changing GxB_sort to modify this behavior though, so your use of GxB_sort to do the compactify will work just fine, for a long time. It might always work, even after I add GrB_IndexBinaryOps.
I think it would be useful to have a "pair"-like binary operator that returns 0 instead of 1. The C spec uses "ONEB" instead of "PAIR", so perhaps "ZEROB" is an okay name.
This is known to be missing in this table: https://github.com/DrTimothyAldenDavis/GraphBLAS/blob/502e16a20317906d1f958e17ef6ac0ec40e5a4af/Include/GraphBLAS.h#L1442-L1468
"Why?", you ask? I want to use
GxB_Vector_sort
andGxB_Matrix_sort
to "compactify" the values to the left or top. When usingGxB_PAIR_BOOL
, the values are reversed. The values would not be reversed withGxB_ZEROB_BOOL
. Is this abusing the sort functions? Maybe :-PI don't see any reason for building predefined semirings with ZEROB instead of PAIR.