[FEA] [REVIEW] RMAT rectangular graph generator #738
Merged
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
…assing it as the offset
…from rmat generator
…rently in cugraph
…rmat-rectangular-generator
…e rectangular cases
…probability distributions are updated for the rectangular case
…rmat-rectangular-generator
teju85
added
enhancement
robertmaynard
approved these changes
Jul 15, 2022
|
@cjnolet did you mean to assign this to someone else for the review? (I'd require a review from somebody as I'm the author of this PR!) |
|
@teju85 I noticed awhile back that cugraph developers have been assigning the authors of the PRs to the PRs themselves as a way to quickly see the faces of the authors in the PRs. I've been doing that as well because I also find it makes it easier to browse the PR list. We don't have to do it in this case. I started looking at this PR yesterday and I'll try finishing up a review today. |
|
Ah I see. Makes sense. Didn't know that. Thanks for letting me know. JFYI, there's an odd failure of the unit-tests in centos. I still need to debug what's going on there. |
cjnolet
approved these changes
Jul 27, 2022
|
@gpucibot merge |
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.
This PR introduces the core RMAT generator primitive into RAFT. It supports the following features:
[a, b; c, d]distribution parameters at each depth.Cc += @cjnolet, JFYI.
Also Cc += @PiotrBigajNV, JFYI.
Note to the reviewers: I know we already have an RMAT generator in cuGraph then why did we have to "duplicate" it in RAFT!? The reason here is that RAFT needs to generate such sparse graphs/adjacency matrices for benchmarking and unit-testing its sparse primitives. Similarly cugraph-ops will also need to do the same in its benchmarks/tests. We cannot introduce cuGraph as a dependency for these things. Hence, this PR. Also, the primitive here is very basic. It only generates the directed edgelist with possible duplicate edges. So, the caller still needs to do this post-processing if/when required. This means, if we are still worried about code duplication, this core logic of cuGraph can be replaced by a call to the proposed API in this PR. I also know that the missing thing here is the
clip_and_flipfeature of cuGraph. It will be added soon.