C API for creating a graph #1907

ChuckHastings · 2021-10-26T20:54:38Z

Partially addresses #1906

This PR defines the API for graph creation and the pagerank and bfs calls that we will use to experiment with transposing a graph.

Some notes on the design here.

The intention is that the C API will handle renumbering (when set to true on graph creation). This means that the opaque cugraph_graph_t pointer being populated by cugraph_sg_graph_create will contain the renumbering device vector and that the C API implementation of algorithms (pagerank and bfs demonstrated here) will unrenumber the result before returning
The intention is that the C API will understand whether the algorithm wants store_transposed=true or store_transposed=false and will call the transpose method if required.

codecov-commenter · 2021-10-27T00:12:18Z

Codecov Report

❗ No coverage uploaded for pull request base (branch-21.12@61b99df). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head ed7b66a differs from pull request most recent head d4a20a7. Consider uploading reports for the commit d4a20a7 to get more accurate results

@@               Coverage Diff               @@
##             branch-21.12    #1907   +/-   ##
===============================================
  Coverage                ?   70.17%           
===============================================
  Files                   ?      143           
  Lines                   ?     8863           
  Branches                ?        0           
===============================================
  Hits                    ?     6220           
  Misses                  ?     2643           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 61b99df...d4a20a7. Read the comment docs.

seunghwak

Looks good in general, and I have few minor comments.

seunghwak · 2021-10-27T17:08:57Z

cpp/include/cugraph_c/algorithms.h

+  bool has_initial_guess,
+  bool do_expensive_check,
+  cugraph_type_erased_device_array_t** vertex_ids,
+  cugraph_type_erased_device_array_t** pageranks);


Why are these "pointer of pointer"?

Recent change.

My recent thought (mentioned in the PR description) is to have renumbering hidden in the C API. With renumbering hidden in the C API, my assumption (unvalidated) is that in a multi-GPU mode I might not know a priori the exact size of the vertex_ids and pageranks array on each node. Thus the memory would be allocated internally.

The functions had been returning a cugraph_type_erased_device_array_t *, making "pointer of pointer" was the easiest way to accommodate this change.

I could make the function require them to be preallocated as input, but you couldn't allocate them on the stack (as currently implemented) because the structure is opaque. They would have to be allocated with the cugraph_type_erased_device_array_create function and then passed in. If the shape of the structure changed (because of renumbering in an MNMG context) they could then be changed in place.

This felt more "C"-like for the pointer to memory allocated inside the function.

seunghwak · 2021-10-27T17:11:27Z

cpp/include/cugraph_c/algorithms.h

+ * @param [out] pageranks   Returns device pointer to pagerank scores
+ * @return error code
+ */
+cugraph_error_t pagerank(


So, we discussed about returning auxiliary information. Not a task for this PR, but we may consider returning something like pagerank_aux_info_t as a wrapper of cugraph_error_t to include additional information later.

I'm leaning toward having all functions either return void or cugraph_error_t - for consistency.

We could pass a pointer to a pagerank_aux_info_t as a parameter for updating auxiliary information.

See

https://github.com/rapidsai/cugraph/pull/1898/files#r737789888
and
https://github.com/rapidsai/cugraph/pull/1898/files#r737799789

We need a consistent strategy to return auxiliary information.

Added some comments to the HITS PR.

That makes sense to me. I'll update the algorithm APIs to reflect this notion.

Just pushed an update that handles this.

cpp/src/c_api/array.cpp

cpp/src/c_api/graph_mg.cpp

cpp/src/c_api/graph_sg.cpp

cpp/CMakeLists.txt

rlratzel

Looks good, I just had a few suggestions.

cpp/include/cugraph_c/algorithms.h

cpp/include/cugraph_c/array.h

cpp/src/c_api/array.cpp

cpp/tests/c_api/c_test_utils.h

cpp/include/cugraph_c/cugraph_api.h

ChuckHastings · 2021-11-02T19:39:50Z

rerun tests

BradReesWork · 2021-11-02T21:32:16Z

@gpucibot merge

ChuckHastings · 2021-11-03T15:43:43Z

rerun tests

rlratzel · 2021-11-04T22:42:16Z

rerun tests

ChuckHastings added 4 commits October 25, 2021 15:38

Add graph creation routines

8791522

add some documentation

aa88d59

add a properties class

adeeba1

debugging

e00d931

ChuckHastings requested review from a team as code owners October 26, 2021 20:54

ChuckHastings self-assigned this Oct 26, 2021

ChuckHastings added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Oct 26, 2021

ChuckHastings added this to the 21.12 milestone Oct 26, 2021

ChuckHastings added 2 commits October 26, 2021 17:12

more documentation, some clang-format issues

d68d4b8

rename cugraph_raft_handle_t to cugraph_handle_t

5c73f3a

seunghwak reviewed Oct 27, 2021

View reviewed changes

address PR review comments

61efaac

ChuckHastings mentioned this pull request Oct 28, 2021

HITS primitive based implementation #1898

Merged

ChuckHastings added 4 commits October 28, 2021 15:48

make return structures for pagerank and bfs, clean up a few things

294921b

fix clang-format issues

8869b80

somehow mangled the copyright header in the last commit

3cbc0e2

fix clang-format issues

b829cfd

ChuckHastings requested a review from aschaffer November 1, 2021 18:24

rlratzel reviewed Nov 1, 2021

View reviewed changes

cpp/include/cugraph_c/algorithms.h Outdated Show resolved Hide resolved

cpp/include/cugraph_c/array.h Show resolved Hide resolved

cpp/src/c_api/array.cpp Show resolved Hide resolved

cpp/tests/c_api/c_test_utils.h Show resolved Hide resolved

aschaffer approved these changes Nov 1, 2021

View reviewed changes

rlratzel reviewed Nov 2, 2021

View reviewed changes

cpp/include/cugraph_c/cugraph_api.h Outdated Show resolved Hide resolved

cpp/include/cugraph_c/cugraph_api.h Outdated Show resolved Hide resolved

ChuckHastings added 4 commits November 2, 2021 11:08

change cugraph_handle_t to cugraph_resource_handle_t

d1dcdd7

add an error object, rename current error to error code

85f7888

add error object to array calls

31e89a0

missed file in last push

72be269

ChuckHastings added 2 commits November 2, 2021 13:42

fix clang-format issues

338cda8

add cugraph_error_t to all of the methods that return error codes

29e6091

rlratzel mentioned this pull request Nov 2, 2021

[ENH] Need to evaluate how to extend C API testing #1923

Open

rlratzel approved these changes Nov 2, 2021

View reviewed changes

ChuckHastings added 2 commits November 2, 2021 15:41

missed file in last commits

3b89785

add bfs to cmake file, fix compilation errors

d4a20a7

seunghwak approved these changes Nov 2, 2021

View reviewed changes

rapids-bot bot merged commit 61e8bad into rapidsai:branch-21.12 Nov 5, 2021

ChuckHastings deleted the fea_c_api_create_graph branch February 1, 2022 16:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

C API for creating a graph #1907

C API for creating a graph #1907

ChuckHastings commented Oct 26, 2021

codecov-commenter commented Oct 27, 2021 •

edited

Loading

seunghwak left a comment

seunghwak Oct 27, 2021

ChuckHastings Oct 27, 2021

seunghwak Oct 27, 2021

ChuckHastings Oct 27, 2021

seunghwak Oct 27, 2021

ChuckHastings Oct 28, 2021

ChuckHastings Oct 28, 2021

rlratzel left a comment

ChuckHastings commented Nov 2, 2021

BradReesWork commented Nov 2, 2021

ChuckHastings commented Nov 3, 2021

rlratzel commented Nov 4, 2021

C API for creating a graph #1907

C API for creating a graph #1907

Conversation

ChuckHastings commented Oct 26, 2021

codecov-commenter commented Oct 27, 2021 • edited Loading

Codecov Report

seunghwak left a comment

Choose a reason for hiding this comment

seunghwak Oct 27, 2021

Choose a reason for hiding this comment

ChuckHastings Oct 27, 2021

Choose a reason for hiding this comment

seunghwak Oct 27, 2021

Choose a reason for hiding this comment

ChuckHastings Oct 27, 2021

Choose a reason for hiding this comment

seunghwak Oct 27, 2021

Choose a reason for hiding this comment

ChuckHastings Oct 28, 2021

Choose a reason for hiding this comment

ChuckHastings Oct 28, 2021

Choose a reason for hiding this comment

rlratzel left a comment

Choose a reason for hiding this comment

ChuckHastings commented Nov 2, 2021

BradReesWork commented Nov 2, 2021

ChuckHastings commented Nov 3, 2021

rlratzel commented Nov 4, 2021

codecov-commenter commented Oct 27, 2021 •

edited

Loading