SPGeMM problem with auto-generated jCuda 12.0 code

[corepointer]

Hi!

With the current code of jCuda 12.0 (#56) the procedure to conduct a sparse matrix-matrix multiplication as described in the Nvidia example code can not be completed (or I am missing something).

The problem seems to be that from the auto-generated code a conflict of const vs non-const sparse matrix descriptors occurs. Some of the involved API functions like cusparseSpGEMM_workEstimation require the output matrix descriptor matC to be of type cusparseSpMatDescr. Makes perfect sense so far as the output needs to be writable of course.

Before the final copy of the output, one needs to query the actual size attributes (rows, cols, nnz) of the result. This is done with the API call to cusparseSpMatGetSize. This one takes a matC of type cusparseConstSpMatDescr. Also makes sense as a query does not need write access.

Now the problem is that in Java code there are two non-convertible types cusparseConstSpMatDescr and cusparseSpMatDescr involved. In the original API the function takes const and optionally non-const:

cusparseStatus_t
cusparseSpMatGetSize(cusparseConstSpMatDescr_t spMatDescr,  // non-const descriptor supported
                     int64_t*                  rows,
                     int64_t*                  cols,
                     int64_t*                  nnz)

Imho two possible solutions would be to either provide a conversion function (internally it's only a nativePointer?) or adapt the code generator to provide const and non-const versions of all the API functions that have this "// non-const descriptor supported" comment.

Regards, Mark

[jcuda]

Thanks for this hint!

Regarding the linked CUDA 12 update issue: I had seen your last comment, but ... could not yet do the release. I am aware that most of the delays are caused by me here, but in hindight, and in this case, this may even have a positive aspect: I think this issue indeed is something that should be solved before releasing the update.

I had seen that they had changed some of the const-handling in this area, but probably didn't sufficiently think through the implications. (IIRC, I even had to manually twiddle the auto-generated code for the const handling - that should have been a red flag... but I was in a hurry)

From a quick glance, the difficulty here is, roughly speaking:

• When a function expects something const, then you can throw in a non-const argument. The function doesn't care.
• The opposite is not true (as long as one is not brutally C-casting away the const, which may cause a real mess...)

And the specific difficulty is that they introduced this ... ("useless"?)... typedef typedef struct cusparseSpMatDescr const* cusparseConstSpMatDescr_t; which makes these things different types, and raises the question of whether there should indeed be two different Java classes for these.

Of the solutions that you proposed,

1. Allow converting the non-const object into a const one to pass to that function
2. Offer two native versions, one for the cusparseSpMatDescr and one for the cusparseConstSpMatDescr

I don't know which one would be "better", from a usage perspective.

Do you have any preference here?

I could imagine that option 2, having two forms of the function, could look a bit redundant (but who cares about that in auto-generated code?), but could be more convenient and more closely mimic the C-API. So I'm leaning towards that solution, but will have to look how I can tweak the code generator accordingly.

[corepointer]

On the one hand I'd go with your decision if you think that can be more easily implemented. On the other hand it could get quite bloated to support all combinations of const and non-const for all the functions.

[jcuda]

On the other hand it could get quite bloated to support all combinations of const and non-const for all the functions.

That's true. (And I don't know whether the >10000 (!) lines that JCusparse.java already has should be a reason for me to say: "It doesn't matter" or "Further bloating should be avoided"...)

But... looking at the documentation, it looks like they used this "const-vs-non-const" pretty inconsistently. For example, cusparseSpMatGetValues is offered in both versions, even though the descriptor is only marked as IN, and to my understanding, it should be possible to only have the const version there as well.

I could try to manually teach the code generator the "right thing to do" here, for each and every function. But given this inconsistency, it probably makes more sense to use the first approach here: I just added a function asConst() to the respective descriptors, via https://github.com/jcuda/jcusparse/commit/0a30b02d7f51516f7280e8e8840609eaf663009f .

I'd have to allocate some time to test it with the specific NVIDIA example code that you referred to. But if you think that this resolves the issue sensibly, then this could be part of the release for CUDA 12 - the corresponding state is tagged as version-12.0.0-RC02 (2!), equal to master at the time of writing this.

[corepointer]

asConst() seems to work just fine afaict from a quick spgemm test :ok_hand: