Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add and test mechanism for creating graph with edge index as weight #2288

Conversation

ChuckHastings
Copy link
Collaborator

Missed this in the last uniform neighborhood sampling updates.

Added a mechanism for creating a graph using an edge index as the weight. This is a temporary mechanism until we add direct support for edge indexes in the graph.

@ChuckHastings ChuckHastings requested a review from a team as a code owner May 19, 2022 05:49
@ChuckHastings ChuckHastings self-assigned this May 19, 2022
@ChuckHastings ChuckHastings added 3 - Ready for Review improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels May 19, 2022
@ChuckHastings ChuckHastings added this to the 22.06 milestone May 19, 2022
Copy link
Contributor

@seunghwak seunghwak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@@ -191,3 +191,102 @@ extern "C" int create_mg_test_graph(const cugraph_resource_handle_t* handle,

return test_ret_value;
}
extern "C" int create_mg_test_graph_with_ids(const cugraph_resource_handle_t* handle,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with_edge_ids might be more intuitive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in latest push

@codecov-commenter
Copy link

Codecov Report

Merging #2288 (d460193) into branch-22.06 (d9ec8f7) will decrease coverage by 0.13%.
The diff coverage is n/a.

@@               Coverage Diff                @@
##           branch-22.06    #2288      +/-   ##
================================================
- Coverage         63.82%   63.69%   -0.14%     
================================================
  Files               100      100              
  Lines              4484     4481       -3     
================================================
- Hits               2862     2854       -8     
- Misses             1622     1627       +5     
Impacted Files Coverage Δ
...n/pylibcugraph/pylibcugraph/utilities/api_tools.py 88.05% <0.00%> (-7.47%) ⬇️
python/cugraph/cugraph/gnn/graph_store.py 80.00% <0.00%> (-2.61%) ⬇️
python/cugraph/cugraph/sampling/node2vec.py 81.81% <0.00%> (ø)
python/cugraph/cugraph/utilities/utils.py 73.79% <0.00%> (+0.86%) ⬆️

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 d9ec8f7...d460193. Read the comment docs.

Copy link
Contributor

@jnke2016 jnke2016 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed and tested. It looks good to me

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b6c111f into rapidsai:branch-22.06 May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement / enhancement to an existing function non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants