Add test to reproduce issue with double weights, fix issue (graph cre…#2305
Conversation
…ation was always treating weights as float)
ChuckHastings
added
3 - Ready for Review
improvement
|
I just ran some tests and it looks like ints as weights are being rejected as well, which causes some of the tests to fail. Is there any reason integer weights aren't supported? Right now it return this error: |
Integer weights are not supported primarily because it would explode the compilation time (which is already very long). Our code uses C++ templates and are templated on the vertex type (int32_t or int64_t), the edge type (int32_t or int64_t), the weight type (float or double), whether or not the graph is stored in transposed format, and whether or not the graph is represented on a single GPU or multiple GPUs. This results in 16 different supported combinations, each of which is generated and compiled. Adding support for integer weight types would double the compile time of the C++ code. To this point we have had very few requests to support integer types. It is certainly something we can add if there is sufficient user interest. |
|
Ok, in that case I will force float/double weights. That must have been what it was doing before if we were never supporting integer weights. |
alexbarghi-nv
left a comment
alexbarghi-nv
left a comment
There was a problem hiding this comment.
LGTM, tests are passing.
| CAPI_EXPECTS((weights == nullptr) || (p_weights->size_ == p_src->size_), | ||
| CUGRAPH_INVALID_INPUT, | ||
| "Invalid input arguments: src size != weights size.", | ||
| *error); |
There was a problem hiding this comment.
Question, weights can be nullptr for either unweighted graphs or weighted but # (local) edges == 0. How do you distinguish these two cases?
There was a problem hiding this comment.
The assumption in the C API is the following:
- For an unweighted graph,
weights == nullptr - For a weighted graph, the weights will be passed, but that it will be an empty array if the number of local edges is 0. So the
cugraph_type-erased_device_array_view_t *would be passed if the graph is weighted, but the data pointer will benullptrand the size will be 0
I will add that to the documentation of the graph creation functions.
There was a problem hiding this comment.
I will also add some unit tests to the creation functions to demonstrate and validate this functionality.
Codecov Report
@@ Coverage Diff @@
## branch-22.06 #2305 +/- ##
================================================
- Coverage 70.82% 63.78% -7.04%
================================================
Files 170 100 -70
Lines 11036 4498 -6538
================================================
- Hits 7816 2869 -4947
+ Misses 3220 1629 -1591
Continue to review full report at Codecov.
|
|
@gpucibot merge |
5 participants
@alexbarghi-nv discovered issue with creating graphs through the C API with double weights.
Added CAPI SSSP test which demonstrates the error. Added fix to the error in the graph creation logic (both SG and MG).