GraphBLAS / graphblas-api-c

Other
7 stars 3 forks source link

Remove C scalars #70

Open jim22k opened 1 year ago

jim22k commented 1 year ago

With the introduction of GrB_Scalar, do we still need C scalars for most functions? Tim D. would like to remove them for everything except getElement and setElement. Doing so will remove lots of complex code which doesn't ultimately do anything useful. It just adds complexity.

Removing these would lower the barrier for any alternate C implementation of the spec.

rayegun commented 1 year ago

I like this a lot. What, however, is the perf impact on SuiteSparse right now? How expensive is the boxing?

Lots of reasons to do this though.

DrTimothyAldenDavis commented 1 year ago

I think the performance impact on SuiteSparse and LAGraph will be minimal. The "boxing" of the plain C scalar into a GrB_Scalar is very cheap.

Code complexity of the plain C scalars is a nightmare. See my _Generic macro for GrB_apply, for example. There are NINETY TWO versions of GrB_apply, if I include my GxB_FC* complex types, or EIGHTY if I exclude them and just count the GrB methods.

All of those must be wrapped into the single GrB_apply macro. Ack.
Try (!) to understand my #define GrB_apply in GraphBLAS.h.

Using GrB_Scalars also might improve future non-blocking exploits.

eriknw commented 1 year ago

If we do this, there should also be a way to get the identity from a monoid. Reducing an empty Vector or Matrix behaves differently whether the result is a C scalar (is monoid identity) or a GrB scalar (is empty).

DrTimothyAldenDavis commented 1 year ago

I think that’s a good idea I’m any case. I have a GxB method that already does that.

DrTimothyAldenDavis commented 1 year ago

The problem of name explosion gets much, much worse, if GxB_eWiseUnion gets added to the spec. That function has 2 input scalars. They can be different types, so that would be 13^2 = 169 entries for built-in C types, the user void, and the GrB_Scalar, or 15^2 = 225 if I include my two complex types. Then there is a Matrix version and Vector version.

Total # of function names with just GrB: 2 169 = 338. With my 2 complex types added: 2 225 = 450.

That's insane. Instead of this, I just have two functions that take in two GrB_Scalars: GxB_Vector_eWiseUnion and GxB_Matrix_eWiseUnion.

eriknw commented 1 year ago

@DrTimothyAldenDavis what are your thoughts on getElement and setElement?

Somewhat related: could/should we have e.g. GrB_Matrix_nvals that uses a GrB_Scalar and accepts a descriptor? Getting nvals implies a computation. Using a GrB_Scalar lets it be delayed, and a descriptor could help control the computation.

DrTimothyAldenDavis commented 1 year ago

I think set and get Element should remain the same.

Getting nvals could require computation so it could use a descriptor. I’m ambivalent about returning a GrB scalar instead of a uint64. I see pros and cons. Maybe have two functions?

DrTimothyAldenDavis commented 1 year ago

Here's a crazy example of how insane GrB_apply is, because of all the C scalars in the spec.

This file (call it ugly.c) won't run, of course, because the matrices are defined, but just look at the C preprocessor output:

#include "GraphBLAS.h"
int main (void)
{
    GrB_Matrix C, M, A ;
    GrB_apply (C, M, NULL, GrB_ABS_FP32, A, NULL) ;
}

Seems tame, right? OK, now look at the output of "cc -E ugly.c" ... where the GrB_apply macro expands to all all 90 variants:

int main (void)
{
    GrB_Matrix C, M, A ;
    _Generic ( (C), GrB_Vector : _Generic ( (GrB_ABS_FP32), GrB_UnaryOp : GrB_Vector_apply , GrB_BinaryOp : _Generic ( (A), const GrB_Scalar: GrB_Vector_apply_BinaryOp1st_Scalar, GrB_Scalar: GrB_Vector_apply_BinaryOp1st_Scalar, const _Bool : GrB_Vector_apply_BinaryOp1st_BOOL, _Bool : GrB_Vector_apply_BinaryOp1st_BOOL, const int8_t : GrB_Vector_apply_BinaryOp1st_INT8, int8_t : GrB_Vector_apply_BinaryOp1st_INT8, const int16_t : GrB_Vector_apply_BinaryOp1st_INT16, int16_t : GrB_Vector_apply_BinaryOp1st_INT16, const int32_t : GrB_Vector_apply_BinaryOp1st_INT32, int32_t : GrB_Vector_apply_BinaryOp1st_INT32, const int64_t : GrB_Vector_apply_BinaryOp1st_INT64, int64_t : GrB_Vector_apply_BinaryOp1st_INT64, const uint8_t : GrB_Vector_apply_BinaryOp1st_UINT8, uint8_t : GrB_Vector_apply_BinaryOp1st_UINT8, const uint16_t : GrB_Vector_apply_BinaryOp1st_UINT16, uint16_t : GrB_Vector_apply_BinaryOp1st_UINT16, const uint32_t : GrB_Vector_apply_BinaryOp1st_UINT32, uint32_t : GrB_Vector_apply_BinaryOp1st_UINT32, const uint64_t : GrB_Vector_apply_BinaryOp1st_UINT64, uint64_t : GrB_Vector_apply_BinaryOp1st_UINT64, const float : GrB_Vector_apply_BinaryOp1st_FP32, float : GrB_Vector_apply_BinaryOp1st_FP32, const double : GrB_Vector_apply_BinaryOp1st_FP64, double : GrB_Vector_apply_BinaryOp1st_FP64, const GxB_FC32_t : GxB_Vector_apply_BinaryOp1st_FC32, GxB_FC32_t : GxB_Vector_apply_BinaryOp1st_FC32, const GxB_FC64_t : GxB_Vector_apply_BinaryOp1st_FC64, GxB_FC64_t : GxB_Vector_apply_BinaryOp1st_FC64, const void * : GrB_Vector_apply_BinaryOp1st_UDT, void * : GrB_Vector_apply_BinaryOp1st_UDT , default: _Generic ( (((void*)0)), const _Bool : GrB_Vector_apply_BinaryOp2nd_BOOL, _Bool : GrB_Vector_apply_BinaryOp2nd_BOOL, const int8_t : GrB_Vector_apply_BinaryOp2nd_INT8, int8_t : GrB_Vector_apply_BinaryOp2nd_INT8, const int16_t : GrB_Vector_apply_BinaryOp2nd_INT16, int16_t : GrB_Vector_apply_BinaryOp2nd_INT16, const int32_t : GrB_Vector_apply_BinaryOp2nd_INT32, int32_t : GrB_Vector_apply_BinaryOp2nd_INT32, const int64_t : GrB_Vector_apply_BinaryOp2nd_INT64, int64_t : GrB_Vector_apply_BinaryOp2nd_INT64, const uint8_t : GrB_Vector_apply_BinaryOp2nd_UINT8, uint8_t : GrB_Vector_apply_BinaryOp2nd_UINT8, const uint16_t : GrB_Vector_apply_BinaryOp2nd_UINT16, uint16_t : GrB_Vector_apply_BinaryOp2nd_UINT16, const uint32_t : GrB_Vector_apply_BinaryOp2nd_UINT32, uint32_t : GrB_Vector_apply_BinaryOp2nd_UINT32, const uint64_t : GrB_Vector_apply_BinaryOp2nd_UINT64, uint64_t : GrB_Vector_apply_BinaryOp2nd_UINT64, const float : GrB_Vector_apply_BinaryOp2nd_FP32, float : GrB_Vector_apply_BinaryOp2nd_FP32, const double : GrB_Vector_apply_BinaryOp2nd_FP64, double : GrB_Vector_apply_BinaryOp2nd_FP64, const GxB_FC32_t : GxB_Vector_apply_BinaryOp2nd_FC32, GxB_FC32_t : GxB_Vector_apply_BinaryOp2nd_FC32, const GxB_FC64_t : GxB_Vector_apply_BinaryOp2nd_FC64, GxB_FC64_t : GxB_Vector_apply_BinaryOp2nd_FC64, const void * : GrB_Vector_apply_BinaryOp2nd_UDT, void * : GrB_Vector_apply_BinaryOp2nd_UDT, default: GrB_Vector_apply_BinaryOp2nd_Scalar ) ), GrB_IndexUnaryOp : _Generic ( (((void*)0)), const _Bool : GrB_Vector_apply_IndexOp_BOOL, _Bool : GrB_Vector_apply_IndexOp_BOOL, const int8_t : GrB_Vector_apply_IndexOp_INT8, int8_t : GrB_Vector_apply_IndexOp_INT8, const int16_t : GrB_Vector_apply_IndexOp_INT16, int16_t : GrB_Vector_apply_IndexOp_INT16, const int32_t : GrB_Vector_apply_IndexOp_INT32, int32_t : GrB_Vector_apply_IndexOp_INT32, const int64_t : GrB_Vector_apply_IndexOp_INT64, int64_t : GrB_Vector_apply_IndexOp_INT64, const uint8_t : GrB_Vector_apply_IndexOp_UINT8, uint8_t : GrB_Vector_apply_IndexOp_UINT8, const uint16_t : GrB_Vector_apply_IndexOp_UINT16, uint16_t : GrB_Vector_apply_IndexOp_UINT16, const uint32_t : GrB_Vector_apply_IndexOp_UINT32, uint32_t : GrB_Vector_apply_IndexOp_UINT32, const uint64_t : GrB_Vector_apply_IndexOp_UINT64, uint64_t : GrB_Vector_apply_IndexOp_UINT64, const float : GrB_Vector_apply_IndexOp_FP32, float : GrB_Vector_apply_IndexOp_FP32, const double : GrB_Vector_apply_IndexOp_FP64, double : GrB_Vector_apply_IndexOp_FP64, const GxB_FC32_t : GxB_Vector_apply_IndexOp_FC32, GxB_FC32_t : GxB_Vector_apply_IndexOp_FC32, const GxB_FC64_t : GxB_Vector_apply_IndexOp_FC64, GxB_FC64_t : GxB_Vector_apply_IndexOp_FC64, const void * : GrB_Vector_apply_IndexOp_UDT, void * : GrB_Vector_apply_IndexOp_UDT, default: GrB_Vector_apply_IndexOp_Scalar ) ), GrB_Matrix : _Generic ( (GrB_ABS_FP32), GrB_UnaryOp : GrB_Matrix_apply , GrB_BinaryOp : _Generic ( (A), const GrB_Scalar: GrB_Matrix_apply_BinaryOp1st_Scalar, GrB_Scalar: GrB_Matrix_apply_BinaryOp1st_Scalar, const _Bool : GrB_Matrix_apply_BinaryOp1st_BOOL, _Bool : GrB_Matrix_apply_BinaryOp1st_BOOL, const int8_t : GrB_Matrix_apply_BinaryOp1st_INT8, int8_t : GrB_Matrix_apply_BinaryOp1st_INT8, const int16_t : GrB_Matrix_apply_BinaryOp1st_INT16, int16_t : GrB_Matrix_apply_BinaryOp1st_INT16, const int32_t : GrB_Matrix_apply_BinaryOp1st_INT32, int32_t : GrB_Matrix_apply_BinaryOp1st_INT32, const int64_t : GrB_Matrix_apply_BinaryOp1st_INT64, int64_t : GrB_Matrix_apply_BinaryOp1st_INT64, const uint8_t : GrB_Matrix_apply_BinaryOp1st_UINT8, uint8_t : GrB_Matrix_apply_BinaryOp1st_UINT8, const uint16_t : GrB_Matrix_apply_BinaryOp1st_UINT16, uint16_t : GrB_Matrix_apply_BinaryOp1st_UINT16, const uint32_t : GrB_Matrix_apply_BinaryOp1st_UINT32, uint32_t : GrB_Matrix_apply_BinaryOp1st_UINT32, const uint64_t : GrB_Matrix_apply_BinaryOp1st_UINT64, uint64_t : GrB_Matrix_apply_BinaryOp1st_UINT64, const float : GrB_Matrix_apply_BinaryOp1st_FP32, float : GrB_Matrix_apply_BinaryOp1st_FP32, const double : GrB_Matrix_apply_BinaryOp1st_FP64, double : GrB_Matrix_apply_BinaryOp1st_FP64, const GxB_FC32_t : GxB_Matrix_apply_BinaryOp1st_FC32, GxB_FC32_t : GxB_Matrix_apply_BinaryOp1st_FC32, const GxB_FC64_t : GxB_Matrix_apply_BinaryOp1st_FC64, GxB_FC64_t : GxB_Matrix_apply_BinaryOp1st_FC64, const void * : GrB_Matrix_apply_BinaryOp1st_UDT, void * : GrB_Matrix_apply_BinaryOp1st_UDT , default: _Generic ( (((void*)0)), const _Bool : GrB_Matrix_apply_BinaryOp2nd_BOOL, _Bool : GrB_Matrix_apply_BinaryOp2nd_BOOL, const int8_t : GrB_Matrix_apply_BinaryOp2nd_INT8, int8_t : GrB_Matrix_apply_BinaryOp2nd_INT8, const int16_t : GrB_Matrix_apply_BinaryOp2nd_INT16, int16_t : GrB_Matrix_apply_BinaryOp2nd_INT16, const int32_t : GrB_Matrix_apply_BinaryOp2nd_INT32, int32_t : GrB_Matrix_apply_BinaryOp2nd_INT32, const int64_t : GrB_Matrix_apply_BinaryOp2nd_INT64, int64_t : GrB_Matrix_apply_BinaryOp2nd_INT64, const uint8_t : GrB_Matrix_apply_BinaryOp2nd_UINT8, uint8_t : GrB_Matrix_apply_BinaryOp2nd_UINT8, const uint16_t : GrB_Matrix_apply_BinaryOp2nd_UINT16, uint16_t : GrB_Matrix_apply_BinaryOp2nd_UINT16, const uint32_t : GrB_Matrix_apply_BinaryOp2nd_UINT32, uint32_t : GrB_Matrix_apply_BinaryOp2nd_UINT32, const uint64_t : GrB_Matrix_apply_BinaryOp2nd_UINT64, uint64_t : GrB_Matrix_apply_BinaryOp2nd_UINT64, const float : GrB_Matrix_apply_BinaryOp2nd_FP32, float : GrB_Matrix_apply_BinaryOp2nd_FP32, const double : GrB_Matrix_apply_BinaryOp2nd_FP64, double : GrB_Matrix_apply_BinaryOp2nd_FP64, const GxB_FC32_t : GxB_Matrix_apply_BinaryOp2nd_FC32, GxB_FC32_t : GxB_Matrix_apply_BinaryOp2nd_FC32, const GxB_FC64_t : GxB_Matrix_apply_BinaryOp2nd_FC64, GxB_FC64_t : GxB_Matrix_apply_BinaryOp2nd_FC64, const void * : GrB_Matrix_apply_BinaryOp2nd_UDT, void * : GrB_Matrix_apply_BinaryOp2nd_UDT, default: GrB_Matrix_apply_BinaryOp2nd_Scalar ) ), GrB_IndexUnaryOp : _Generic ( (((void*)0)), const _Bool : GrB_Matrix_apply_IndexOp_BOOL, _Bool : GrB_Matrix_apply_IndexOp_BOOL, const int8_t : GrB_Matrix_apply_IndexOp_INT8, int8_t : GrB_Matrix_apply_IndexOp_INT8, const int16_t : GrB_Matrix_apply_IndexOp_INT16, int16_t : GrB_Matrix_apply_IndexOp_INT16, const int32_t : GrB_Matrix_apply_IndexOp_INT32, int32_t : GrB_Matrix_apply_IndexOp_INT32, const int64_t : GrB_Matrix_apply_IndexOp_INT64, int64_t : GrB_Matrix_apply_IndexOp_INT64, const uint8_t : GrB_Matrix_apply_IndexOp_UINT8, uint8_t : GrB_Matrix_apply_IndexOp_UINT8, const uint16_t : GrB_Matrix_apply_IndexOp_UINT16, uint16_t : GrB_Matrix_apply_IndexOp_UINT16, const uint32_t : GrB_Matrix_apply_IndexOp_UINT32, uint32_t : GrB_Matrix_apply_IndexOp_UINT32, const uint64_t : GrB_Matrix_apply_IndexOp_UINT64, uint64_t : GrB_Matrix_apply_IndexOp_UINT64, const float : GrB_Matrix_apply_IndexOp_FP32, float : GrB_Matrix_apply_IndexOp_FP32, const double : GrB_Matrix_apply_IndexOp_FP64, double : GrB_Matrix_apply_IndexOp_FP64, const GxB_FC32_t : GxB_Matrix_apply_IndexOp_FC32, GxB_FC32_t : GxB_Matrix_apply_IndexOp_FC32, const GxB_FC64_t : GxB_Matrix_apply_IndexOp_FC64, GxB_FC64_t : GxB_Matrix_apply_IndexOp_FC64, const void * : GrB_Matrix_apply_IndexOp_UDT, void * : GrB_Matrix_apply_IndexOp_UDT, default: GrB_Matrix_apply_IndexOp_Scalar ) ) ) (C, M, ((void*)0), GrB_ABS_FP32, A, ((void*)0)) ;
}

Somebody had to write that nasty macro:

https://github.com/DrTimothyAldenDavis/GraphBLAS/blob/502e16a20317906d1f958e17ef6ac0ec40e5a4af/Include/GraphBLAS.h#L7632-L7678

which also uses a helper macro that appears in many places:

https://github.com/DrTimothyAldenDavis/GraphBLAS/blob/502e16a20317906d1f958e17ef6ac0ec40e5a4af/Include/GraphBLAS.h#L682-L711

It's doable but really insane, and this kind of code (required by the spec) is a barrier to the creation of other GraphBLAS implementations.