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

Dgl graph store #2046

Merged
merged 9 commits into from
Feb 2, 2022
Merged

Dgl graph store #2046

merged 9 commits into from
Feb 2, 2022

Conversation

BradReesWork
Copy link
Member

new GraphStore for DGL intergration
Code will need to be added to DGL

@BradReesWork BradReesWork requested a review from a team as a code owner January 27, 2022 22:22
@BradReesWork BradReesWork added this to the 22.04 milestone Jan 27, 2022
@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jan 27, 2022
python/cugraph/cugraph/gnn/graph_store.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/gnn/graph_store.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/gnn/graph_store.py Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Feb 2, 2022

Codecov Report

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

Impacted file tree graph

@@               Coverage Diff               @@
##             branch-22.04    #2046   +/-   ##
===============================================
  Coverage                ?   73.27%           
===============================================
  Files                   ?      154           
  Lines                   ?     9947           
  Branches                ?        0           
===============================================
  Hits                    ?     7289           
  Misses                  ?     2658           
  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 afeebee...c786ae1. Read the comment docs.

@BradReesWork
Copy link
Member Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b2b9eb9 into rapidsai:branch-22.04 Feb 2, 2022
@wangxiaoyunNV
Copy link
Contributor

wangxiaoyunNV commented Feb 2, 2022

Hi Brad,
When I review the graphStorage class, I notice here is no output_device parameter in the sampling functions. But the DGL graphstorage sampling APIs have output_device parameter. Andrei's code has that as well. Could wee add that parameter to the samplings functions? or have another function called to_device in the GraphStorage class?
https://github.com/dmlc/dgl/blob/701b4fccc2eed979ae3db801fabb6bf7bc03940c/examples/pytorch/__temporary__/dglnew/graph/graph.py

Thank you,
Xiaoyun

Copy link
Contributor

@wangxiaoyunNV wangxiaoyunNV left a comment

Choose a reason for hiding this comment

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

I have a concern about CuGraphStore class, it should have a to_device function or parameter in the sampling functions.

Thank you!

max_depth=length, use_padding=True)

return p, w, s

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also have a function like async_fetch(self, ids, device): in the cuGraphStore class? so we can send the sampled graphs to different GPUs.
Thank you!

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.

4 participants