GraphBLAS / graphblas-api-c

Other
7 stars 3 forks source link

GxB_INDEX_MAX #34

Closed DrTimothyAldenDavis closed 3 years ago

DrTimothyAldenDavis commented 3 years ago

I define GxB_INDEX_MAX as 2^60, which defines the largest dimension of any matrix or vector I can allow. UINT64_MAX is too big. I suggest that each library defines its own GrB_INDEX_MAX.

mcmillan03 commented 3 years ago

I am tasked with adding this to the spec but I believe there is a naming issue...

I suspect that SuiteSparse's 1<<60 is not GrB_MAX_INDEX but rather GrB_MAX_NUM_VERTICES. These are different concepts.

MAX_INDEX should be the maximum allowable index which is 1 less (in 0-based index) than maximum number of vertices.

Further it seems clearer to me to require definition of MAX_INDEX rather than MAX_NUM_VERTICES.

DrTimothyAldenDavis commented 3 years ago

Good point. You're correct, my GxB_INDEX_MAX is 2^60 and this is actually my MAX_NUM_VERTICES. See for example my test in GrB_Matrix_new:

https://github.com/DrTimothyAldenDavis/GraphBLAS/blob/64f6971169877eecc601b23d7115aef8d6b3ea07/Source/GrB_Matrix_new.c#L35

For me, then, GrB_INDEX_MAX would be ((1<<60)-1).

DrTimothyAldenDavis commented 3 years ago

If GrB_MAX_INDEX is added to the spec, I would set it as:

define GrB_MAX_INDEX ((GrB_index) (1ULL << 60) - 1)

and then I would revise my tests in GrB_Matrix_new and GrB_Vector_new accordingly.

I have a slight preference for GrB_INDEX_MAX, since it follows the form of the ANSI C11 INT64_MAX and UINT64_MAX.

DrTimothyAldenDavis commented 3 years ago

I recall why I used the name GxB_INDEX_MAX while at the same time using the same number as the max matrix dimensions. The definition of GrB_Matrix_new is:

GrB_Info GrB_Matrix_new (GrB_Matrix *A, GrB_Type type, GrB_Index nrows, GrB_Index ncols) ;

The type of nrows and ncols is GrB_Index, not something like GrB_Dimension, and I think of GxB_INDEX_MAX as the maximum value permitted for this data type. That means my GxB_INDEX_MAX is the maximum row and column dimension of a matrix. The indices inside such a matrix would then range from 0 to GxB_INDEX_MAX-1.

mcmillan03 commented 3 years ago

Currently, the valid index ranges for a matrix built with GrB_Matrix_new are [0, nrows) x [0, ncols) (it is invalid for nrows or ncols to be zero).

So nrows and ncols are not "MAX_INDEX" values, but rather "NUM_VERTICES" values. So if GrB_MAX_INDEX is 2^64-1 there is no way to specify a matrix of the maximum size using GrB_Matrix_new().

I think it is dangerous to change the meaning of GrB_Matrix_new parameters to be max_row_index and max_col_index.

DrTimothyAldenDavis commented 3 years ago

Right, except a value of GrB_INDEX_MAX of (2^64)-1, or UINT64_MAX, would break the integer index computations in the GrB_IndexUnaryOps. The largest possible value an index can be would be INT64_MAX.

mcmillan03 commented 3 years ago

Do we need a compile time macro and a runtime constant?

DrTimothyAldenDavis commented 3 years ago

I think a compile-time macro is enough. Mine would be

#define GrB_INDEX_MAX ((1ULL<<60)-1)

mcmillan03 commented 3 years ago

Cleaned up wording. Added error text to Matrix/Vector_new regarding valid values of nsize, nrows, and ncols.