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

Replace PropertyGraph in cugraph-PyG with FeatureStore #3159

Merged
merged 380 commits into from
Jan 27, 2023

Conversation

alexbarghi-nv
Copy link
Member

@alexbarghi-nv alexbarghi-nv commented Jan 19, 2023

Replaces PropertyGraph in cugraph-PyG with the new FeatureStore, which is the preferred solution for storing tensor features.
Closes #3151
Closes #3079

alexbarghi-nv and others added 30 commits November 23, 2022 08:57
… with the server, added tests which use/test the new builtin test extensions.
…s (via rapids-pytest-benchmark plugin) easier to read, changes to allow RemoteGraph to use either a server-side Graph or PropertyGraph.
…r LocalCUDACluster for MG service, re-wrote start list generation to work with both SG and MG, added server extension for generating start list, changed scale from 24 to 23 to prevent OOM, added separate function for creating MG graphs, added debug print to show result sizes, fixes to cugraph MG Graph for incorrect column names, added util to cugraph for starting and stopping dask client.
…art so it fails faster, including the stdout/stderr output in the exception raised or to console depending on how it exited.
@BradReesWork BradReesWork modified the milestones: 23.02, 23.04 Jan 23, 2023
@alexbarghi-nv alexbarghi-nv removed request for a team January 23, 2023 23:54
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.

Minor review on using cuda-array-interface

python/cugraph-pyg/cugraph_pyg/data/cugraph_store.py Outdated Show resolved Hide resolved
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

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.

Noticed one minor item which need not hold up approval.

python/cugraph-pyg/cugraph_pyg/data/__init__.py Outdated Show resolved Hide resolved
@rlratzel
Copy link
Contributor

/merge

@codecov-commenter
Copy link

Codecov Report

Base: 55.22% // Head: 55.31% // Increases project coverage by +0.08% 🎉

Coverage data is based on head (78b7e57) compared to base (5c021fb).
Patch coverage: 77.63% of modified lines in pull request are covered.

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-23.02    #3159      +/-   ##
================================================
+ Coverage         55.22%   55.31%   +0.08%     
================================================
  Files               148      148              
  Lines              9543     9553      +10     
================================================
+ Hits               5270     5284      +14     
+ Misses             4273     4269       -4     
Impacted Files Coverage Δ
python/cugraph-pyg/cugraph_pyg/data/__init__.py 100.00% <ø> (ø)
...ugraph/cugraph/gnn/feature_storage/feat_storage.py 83.67% <50.00%> (-1.44%) ⬇️
...thon/cugraph-pyg/cugraph_pyg/data/cugraph_store.py 76.43% <77.56%> (-3.78%) ⬇️
...cugraph-pyg/cugraph_pyg/sampler/cugraph_sampler.py 80.32% <100.00%> (+41.26%) ⬆️
...hon/cugraph-pyg/cugraph_pyg/utilities/api_tools.py 29.16% <0.00%> (-5.21%) ⬇️

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.

@rapids-bot rapids-bot bot merged commit 6e9b518 into rapidsai:branch-23.02 Jan 27, 2023
rapids-bot bot pushed a commit that referenced this pull request Jan 31, 2023
Closes #3152 
Depends on #3148 and #3159 
This PR does support multiple trainers, even though there is no example for it.

Authors:
  - Alex Barghi (https://github.com/alexbarghi-nv)
  - Chuck Hastings (https://github.com/ChuckHastings)
  - Vibhu Jawa (https://github.com/VibhuJawa)

Approvers:
  - Vibhu Jawa (https://github.com/VibhuJawa)
  - Rick Ratzel (https://github.com/rlratzel)

URL: #3170
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change improvement Improvement / enhancement to an existing function
Projects
None yet
7 participants