Skip to content

Add RelGraphConv, GATConv and SAGEConv models to cugraph_dgl#3131

Merged
rapids-bot[bot] merged 11 commits intorapidsai:branch-23.02from
tingyu66:add-nn-conv
Jan 20, 2023
Merged

Add RelGraphConv, GATConv and SAGEConv models to cugraph_dgl#3131
rapids-bot[bot] merged 11 commits intorapidsai:branch-23.02from
tingyu66:add-nn-conv

Conversation

@tingyu66
Copy link
Member

@tingyu66 tingyu66 commented Jan 11, 2023 •
edited
Loading

This PR adds RGCN, GAT and GraphSAGE models that are accelerated by cugraph-ops.

@tingyu66 tingyu66 requested review from a team as code owners January 11, 2023 20:22
@tingyu66 tingyu66 requested a review from VibhuJawa January 11, 2023 20:23
@tingyu66 tingyu66 added 3 - Ready for Review non-breaking Non-breaking change feature request New feature or request labels Jan 11, 2023
@tingyu66
Copy link
Member Author

tingyu66 commented Jan 12, 2023

rerun tests

VibhuJawa
VibhuJawa suggested changes Jan 12, 2023
View reviewed changes
Copy link
Member

@VibhuJawa VibhuJawa left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Requested some changes to make torch optional import , the reason for optional importing is compatibility of building with DLFW containers.

@tingyu66
Copy link
Member Author

tingyu66 commented Jan 12, 2023

Requested some changes to make torch optional import , the reason for optional importing is compatibility of building with DLFW containers.

Gotcha, thanks for the feedback. Now it uses import_optional() throughout.

@tingyu66
Copy link
Member Author

tingyu66 commented Jan 12, 2023 •
edited
Loading

This PR resolves #3137.

@tingyu66 tingyu66 added this to the 23.02 milestone Jan 12, 2023
@rlratzel rlratzel linked an issue Jan 13, 2023 that may be closed by this pull request
Add GraphSAGE and RGCN model to cugraph-dgl package #3137
Closed
VibhuJawa
VibhuJawa approved these changes Jan 14, 2023
View reviewed changes
Copy link
Member

@VibhuJawa VibhuJawa left a comment

Choose a reason for hiding this comment

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

LGTM

@rlratzel
Copy link
Contributor

rlratzel commented Jan 17, 2023

rerun tests

@rlratzel rlratzel mentioned this pull request Jan 17, 2023
Closed
2 tasks
@ajschmidt8
Copy link
Member

ajschmidt8 commented Jan 18, 2023

rerun tests

Just a reminder that this command doesn't work for GH Actions. Please check the docs below on rerunning tests for GH Actions.

ajschmidt8
ajschmidt8 approved these changes Jan 18, 2023
View reviewed changes
Copy link
Member

@ajschmidt8 ajschmidt8 left a comment

Choose a reason for hiding this comment

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

Approving ops-codeowner file changes

@tingyu66 tingyu66 marked this pull request as draft January 20, 2023 03:45
@tingyu66 tingyu66 changed the title Add RelGraphConv and SAGEConv models to cugraph_dgl Add RelGraphConv, GATConv and SAGEConv models to cugraph_dgl Jan 20, 2023
@tingyu66 tingyu66 marked this pull request as ready for review January 20, 2023 07:07
VibhuJawa
VibhuJawa reviewed Jan 20, 2023
View reviewed changes
rlratzel
rlratzel reviewed Jan 20, 2023
View reviewed changes
Copy link
Contributor

@rlratzel rlratzel left a comment

Choose a reason for hiding this comment

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

One minor thing jumped out which I asked about.

However, we're also trying to use type annotations in new public APIs. Would those be relatively easy to add, or should we file an issue to do that later?

@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2023 •
edited
Loading

Codecov Report

Base: 55.31% // Head: 55.31% // No change to project coverage 👍

Coverage data is based on head (ca2a32e) compared to base (ea132d3).
Patch has no changes to coverable lines.

Additional details and impacted files 
@@              Coverage Diff              @@
##           branch-23.02    #3131   +/-   ##
=============================================
  Coverage         55.31%   55.31%           
=============================================
  Files               148      148           
  Lines              9423     9423           
=============================================
  Hits               5212     5212           
  Misses             4211     4211

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@tingyu66 tingyu66 mentioned this pull request Jan 20, 2023
Closed
@rlratzel
Copy link
Contributor

rlratzel commented Jan 20, 2023

/merge

@rapids-bot rapids-bot bot merged commit 38cdae9 into rapidsai:branch-23.02 Jan 20, 2023
@tingyu66 tingyu66 deleted the add-nn-conv branch January 21, 2023 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rlratzel rlratzel rlratzel approved these changes

@ajschmidt8 ajschmidt8 ajschmidt8 approved these changes

+1 more reviewer

@VibhuJawa VibhuJawa VibhuJawa approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

feature request New feature or request non-breaking Non-breaking change

Projects

None yet

Milestone

23.02

Development

Successfully merging this pull request may close these issues.

Add GraphSAGE and RGCN model to cugraph-dgl package

5 participants

@tingyu66 @rlratzel @ajschmidt8 @codecov-commenter @VibhuJawa