GraphBLAS / graphblas-api-c

Other
7 stars 3 forks source link

BB-4: Macro to identify the library implementation #2

Open mcmillan03 opened 3 years ago

mcmillan03 commented 3 years ago

Each library should define a macro that declares itself to the user, such as GrB_XXX_GRAPHBLAS, where XXX is the name of implementation. So the user can write this, if needed:

#if defined ( GrB_SUITESPARSE_GRAPHBLAS )
...
#elif defined ( GrB_GBTL_GRAPHBLAS )
...
#elif defined ( GrB_GUNROCK_GRAPHBLAS )
...
#endif
tgmattso commented 3 years ago

We need a macro GrB_IMPLEMENTATION and GrB_VERSION or GrB_DATE

We also need to think about the platform issue so I can mix two different GraphBLAS libraries in a single program.

mcmillan03 commented 3 years ago

GrB_VERSION already in use (it is the spec version implemented)...GrB_IMPLEMENTATION_VERSION perhaps.

DrTimothyAldenDavis commented 3 years ago

This very important, as is a run-time query for the name and version of the library. I run into linking problems because SS:GraphBLAS is too successful...

I can accidentally link against a libgraphblas.so version 1.0 that comes built in to Linux. It's my own library, SuiteSparse:GraphBLAS, but version 1.0. MATLAB has SuiteSparse:GraphBLAS v3.3 built in.

If I can query the name and version of the library at both compile-time and at run time, then I can more easily catch these errors. Adding this functionality as both macros and user-callable functions (both) is simple to do but would save a lot of headache.

I don't recommend allowing the mix of 2 different GraphBLAS libraries in a single program. Who owns GrB_mxm? Is it me or someone else? C has no namespace management like C++ has.

manoj63 commented 1 year ago

Isn't the suggestion contrary to the very idea of having a standard API for portability of application across libraries being used? Are we giving application programmers freedom to implement non-portable code by putting library specific calls?

The question really is whether the C-API version (such as 2.0, 2.1, ...) supported by the library needs query-able, in which case it should be a function call.

rayegun commented 1 year ago

As mentioned in last committee meeting we must have this feature in get/set in order to support implementation specific hints/fields. Without that we can easily get silent wrong behavior if two extensions GxB_MYFIELD and GyB_OTHERFIELD use the same enum value.

While yes, it's ideal to avoid implementation specific behavior we can't be too strict and suffocate extensions to the spec.

For instance Tim has a type hint for bitmap, which we won't be adding to the spec. If another library defines a type hint for ELL or DIA and accidentally GxB_BITMAP == GyB_DIA then we have at minimum a silent performance problem.

manoj63 commented 1 year ago

Will, that is the question. Doesn't implementation specificity make the program non-portable across graphBLAS libraries, and constraint the libraries from evolving. I think we just need to be careful about what we are getting and setting.

For getting C_API version, GrB_getVersion( ) is there.

rayegun commented 1 year ago

No. Implementations are already non-portable, and Julia, pygraphblas, and LAGraph are non-portable due (in part) to the lack of this feature.

The purpose is to allow users and other library authors to ensure that extensions can be used portably. That is you can say:

if GrB_get(GrB_IMPLEMENTATION) == "SuiteSparse"
    <use extension>
elseif GrB_get(GrB_IMPLEMENTATION) == "GraphBLAST"
    <use extension>
else
    <use default spec compliant behavior>
end

We should define both a compile time macro and a runtime GrB_get to ensure that users can opt in to extensions.

As it stands now one cannot use SuiteSparse extensions, or more importantly, GrB_FIELD extensions, without failing to compile or even crashing with dynamic linking against another implementation. If we bury our heads in the sand on this we will make it impossible to use extensions as simple as GrB_FIELD, or extended built-in semirings (of which Tim has many).

jim22k commented 1 year ago

This will be part of the get/set proposal, but also needs a macro.