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

Use new sampling primitives #2751

Conversation

ChuckHastings
Copy link
Collaborator

@ChuckHastings ChuckHastings commented Sep 28, 2022

Closes #2581
Closes #2582
Closes #2665

Update the neighborhood sampling algorithm to use the new neighborhood sampling primitive defined in #2703

@ChuckHastings ChuckHastings requested a review from a team as a code owner September 28, 2022 00:02
@ChuckHastings ChuckHastings self-assigned this Sep 28, 2022
@ChuckHastings ChuckHastings added 2 - In Progress improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 28, 2022
@ChuckHastings ChuckHastings added this to the 22.10 milestone Sep 28, 2022
@ChuckHastings ChuckHastings requested a review from a team as a code owner September 30, 2022 18:24
@ChuckHastings ChuckHastings changed the title [skip-ci] Use new sampling primitives Use new sampling primitives Sep 30, 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.

Looks like a big improvement in simplifying our code to support neighbor sampling and random walks.

I feel like we can delete many more detail space sampling related files and further simplify the codebase, but I guess we can revisit this in the next release.

@codecov-commenter
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@e852b3c). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-22.10    #2751   +/-   ##
===============================================
  Coverage                ?   59.77%           
===============================================
  Files                   ?      111           
  Lines                   ?     6160           
  Branches                ?        0           
===============================================
  Hits                    ?     3682           
  Misses                  ?     2478           
  Partials                ?        0           

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.

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 83f27a1 into rapidsai:branch-22.10 Oct 3, 2022
22.10 Release automation moved this from PRs to Done Oct 3, 2022
rapids-bot bot pushed a commit that referenced this pull request Oct 11, 2022
This PR fixes the below errors that have popped up in MNMG testing. 
- [x] fix_out of index keys on MNMG graphs
- [x] fix loc/get_node_storage error  on MNMG graphs 
(Work around  rapidsai/cudf#11877)
- [x] Clear Cached Properties when they become invalid
- [x] Remove  6 pytest skipping as both these PRs have landed 
- #2751
- #2523
- [x] Change `vertex_col_names` to  `node_col_names`  to match DGL 
- [x] Ensure MNMG tests pass 
- [x] Work around the PG bug and also prevent redundant conversion to lists

Authors:
  - Vibhu Jawa (https://github.com/VibhuJawa)

Approvers:
  - Rick Ratzel (https://github.com/rlratzel)
  - Erik Welch (https://github.com/eriknw)

URL: #2786
@ChuckHastings ChuckHastings deleted the fea_use_new_sampling_primitives branch December 2, 2022 18:36
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
No open projects
22.10 Release
  
Done
5 participants