Closed DrTimothyAldenDavis closed 3 years ago
DrTimothyAldenDavis
commented
3 years ago The boolean ROWLE operator is i <= thunk. The boolean ROWGT operator is i > thunk
If this change is made to ROWINDEX, then when the result is typecast to bool, the operator becomes i != thunk, which is like "ROWNE" and it fits nicely with ROWLE and ROWGT when all 3 are used by GrB_select.
mcmillan03
commented
3 years ago The IndexUnaryOps that don't return bool aren't really intended for select (sure you can get some interesting behaviour as you point out). If ROWEQ and ROWNE (and COL*) selectors are needed then they should be added to predefined set rather than require someone to reason about how to use something like ROWINDEX function for the same thing. Further, the code at the call site would not be clear.
DrTimothyAldenDavis
commented
3 years ago Ok. Let’s leave them as is then. Let’s not add more ops
The ROWINDEX operator returns the integer value i+thunk for some scalar thunk. If this operator is used in GrB_select, the result is typecasted to a boolean: (i+thunk) != 0, which then becomes i != -thunk. In other words, the operator says "select the entire matrix except delete any rows in row -thunk".
That seems backwards, in the sense that if you want to delete row 5, you would need to use:
GrB_select (C, ... GrB_ROWINDEX_INT64, A, -5, ... )
It seems to me that the value should be passed in as 5, not -5. The ROWINDEX operator could then be (i-thunk). Likewise the built-in COLINDEX operator should be (j-thunk).
This points out another problem with the built-in GrB_IndexUnaryOps, as well. If ROWINDEX_T is left as-is, thunk must be negative. That's fine for INT64, but broken for UINT64 since thunk cannot be negative in that case. There should be no built-in GrB_IndexUnaryOps with the UINT32 or UINT64 suffix. Most of these built-in index computations require the ability to work with negative numbers. If ROWINDEX is changed, thunk could be unsigned but then i-thunk is not defined if i < thunk. So in both cases, ROWINDEX doesn't work with unsigned integers.