search

GraphBLAS / graphblas-api-c

Other
7 stars 3 forks source link

TRIL and TRIU are off-by-one when compared to the MATLAB functions of the same name #40

Closed DrTimothyAldenDavis closed 2 years ago

DrTimothyAldenDavis commented 2 years ago

The names TRIL and TRIU for the biult-in GrB_IndexUnaryOps derive from the MATLAB functions of the same name, but they are off by one from MATLAB.

In MATLAB, TRIL (A, 0) returns entries on or below the main diagonal (k = 0). TRIU (A, 0) returns entries on or above the main diagonal. The 2nd parameter is the "thunk" scalar. These tests correspond to (j-i) <= thunk for TRIL, and (j-i) >= thunk for TRIU.

But in the GrB_IndexUnaryOps, TRIL is defined as j < (i+thunk), which is the same as (j-i) < thunk. As a result, TRIL(A,0) in MATLAB is TRIL(A,1) in GraphBLAS. This will cause confusion, since it's off by just one.

I suggest that the TRIL operator be defined as (j-i) <= thunk, or (j-i) <= s using the notation in the spec. TRIU should be (j-i) >= s.

mcmillan03 commented 2 years ago

Modifying as follows: TRIL: j <= i + s on or below diagonal s TRIU: j >= i + s on or above diagonal s

DrTimothyAldenDavis commented 2 years ago

Great! I'll update my draft implementation accordingly.