Remove Dummy Edge Weights, Support Specifying Edge Ids/Edge Types/Weights Separately #3495
Conversation
alexbarghi-nv
added
improvement
…to select-edge-props
…s to support unweighted graphs
cpp/src/c_api/graph_mg.cpp
Outdated
| @@ -338,19 +338,19 @@ extern "C" cugraph_error_code_t cugraph_mg_graph_create( | |||
| weight_type = cugraph_data_type_id_t::FLOAT32; | |||
| } | |||
|
|
|||
| CAPI_EXPECTS((edge_type_ids == nullptr) || (p_edge_ids->type_ == edge_type), | |||
| CAPI_EXPECTS((edge_ids == nullptr) || (p_edge_ids->type_ == edge_type), | |||
| CUGRAPH_INVALID_INPUT, | |||
| "Invalid input arguments: Edge id type must match edge (src/dst) type", | |||
There was a problem hiding this comment.
Is this comment accurate?
Edge ID should match with edge_t, not vertex_t.
cpp/src/c_api/graph_sg.cpp
Outdated
| *error); | ||
| CAPI_EXPECTS((edge_ids == nullptr) || (p_edge_ids->type_ == edge_type), | ||
| CUGRAPH_INVALID_INPUT, | ||
| "Invalid input arguments: Edge id type must match edge (src/dst) type", |
There was a problem hiding this comment.
Same here. edge ID type should match with edge_t, not vertex_t.
There was a problem hiding this comment.
I changed the message, but it's still confusing to the user why they may get an error if their input graph has int64 vertex ids but only int32 edge ids. It's not that crazy for that to happen (i.e. vertex ids are a hash). I'll consider adding something at the Python layer to make that more clear.
There was a problem hiding this comment.
I just added the warning at the Python layer.
There was a problem hiding this comment.
it's still confusing to the user why they may get an error if their input graph has int64 vertex ids but only int32 edge ids.
Yeah, this needs some investigation.
One reason is, we're using vertex_t for both original & renumbered vertex IDs. Another reason is to limit the number of explicit template instantiations (this affects compile time & binary size).
After renumbering, # edges is generally more than # vertices (not true for graphs with many isolated vertices, but this is rather an exception).
…cugraph into select-edge-props
|
/merge |
1 similar comment
|
/merge |
Resolves #3318 Resolves #3385 Updates the bulk sampler to use the offsets returned by the `uniform_neighbor_sample` call, and take advantage of the pre-partitioned data returned from C++ to avoid repartitioning and scanning, and also to reduce the number of files written. This PR is expected to significantly improve memory and compute time for bulk sampling. Merge after #3517 - Merged Merge after #3495 - Merged Authors: - Alex Barghi (https://github.com/alexbarghi-nv) - Chuck Hastings (https://github.com/ChuckHastings) - Brad Rees (https://github.com/BradReesWork) Approvers: - Rick Ratzel (https://github.com/rlratzel) URL: #3524
Closes #3486
Merge after #3513 - Merged