[FEA] Support Seed Retention for Sampling with Renumbering #4355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…to bug_post_processing
…to bug_post_processing
…to bug_post_processing
…to bug_post_processing
…to bug_post_processing
…to bug_post_processing
…to bug_post_processing
alexbarghi-nv
added
feature request
…i-nv/cugraph into support-seed-retention
naimnv
reviewed
May 6, 2024
ChuckHastings
approved these changes
May 6, 2024
This is not an added C++ option, it already existed. It was never exposed/used through the C API. I believe (didn't exhaustively look) that it is already tested in |
naimnv
approved these changes
May 6, 2024
|
/merge |
rlratzel
approved these changes
May 14, 2024
Comment on lines
+151
to
+154
| label_offsets: integer sequence, optional (default=None) | ||
| Offsets of each label within the start vertex list. | ||
| Only used if retain_seeds is True. Required if retain_seeds | ||
| is True. |
There was a problem hiding this comment.
Just a suggestion: maybe change this so that if this is specified, the retain_seeds=True behavior is assumed and then drop the retain_seeds arg completely? If you decide to do that, then be sure to also include the description of the retain_seeds behavior in this description.
There was a problem hiding this comment.
That's a reasonable suggestion. I'll add that to the new PR.
| @@ -342,13 +353,15 @@ def uniform_neighbor_sample( | |||
| else None, | |||
| h_fan_out=fanout_vals, | |||
| with_replacement=with_replacement, | |||
| do_expensive_check=False, | |||
| do_expensive_check=True, | |||
There was a problem hiding this comment.
not intentional, whoops. We need to fix that.
rapids-bot
bot
merged commit 45371cb
into
rapidsai:branch-24.06
130 of 131 checks passed
This was referenced May 14, 2024
rapids-bot bot
pushed a commit
that referenced
this pull request
May 15, 2024
…ample (#4421) #4355 accidentally added an expensive check to SG uniform neighbor sample. This PR reverts that addition. Authors: - Alex Barghi (https://github.com/alexbarghi-nv) Approvers: - Rick Ratzel (https://github.com/rlratzel) URL: #4421
rapids-bot bot
pushed a commit
that referenced
this pull request
May 30, 2024
Distributed sampling in cuGraph-PyG. Also renames the existing API to clarify that it is dask based. Adds a dependency on `tensordict` for `cuGraph-PyG` which supports the new `TensorDictFeatureStore`. Also no longer installs `torch-cluster` and `torch-spline-conv` in CI for testing since that results in an `ImportError` and neither of those packages are needed. Requires PyG 2.5. Should be merged after #4335 Merge after #4355 Closes #4248 Closes #4249 Closes #3383 Closes #3942 Closes #3836 Closes #4202 Closes #4051 Closes #4326 Closes #4252 Partially addresses #3805 Authors: - Alex Barghi (https://github.com/alexbarghi-nv) - Seunghwa Kang (https://github.com/seunghwak) - Tingyu Wang (https://github.com/tingyu66) - Ralph Liu (https://github.com/nv-rliu) Approvers: - Tingyu Wang (https://github.com/tingyu66) - Brad Rees (https://github.com/BradReesWork) - Jake Awe (https://github.com/AyodeAwe) URL: #4384
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Exposes the ability to retain seeds even if they have no outgoing edges (and therefore are not sampled). Required to fix the current bug in cuGraph-PyG involving batch size and dropping seeds.
Currently, this functionality can't be exposed through the MG Python API (#4358) but exposing it through the pylibcugraph API is sufficient to resolve this issue. This PR does expose it through the SG Python API.