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.
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
Add R-mat generator #1411
Add R-mat generator #1411
Changes from 7 commits
ac24a8d
3c89ba3
f74c3ff
f238f2f
279187c
afad41e
b72af3e
8c10e51
1b6b201
0a29f2c
3331999
2526918
48fdcd3
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way I understand this is that when
clip_and_flip
is false this returns a directed graph. When it is true it can be seen as an undirected one but not the kind of format we use in cugraph.I think we would benefit from exposing an option that generates the undirected graph inputs that we expect in cugraph.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is correct (if clip-and-flip is set to true, all the edges are in the lower triangular part of the graph adjacency matrix, they need to be symmetrized for cuGraph use), and my plan is to symmetrize the output after this R-mat generator; so to keep the R-mat generator's behavior close to Graph 500. Any thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine with doing symmetrize separately, I just recall that the symmetrize step was done at the python level with cudf (concat + group_by) before. Do we have that feature at C++ level now? If not, we would need to add it before we can use this for C++ testing for instance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we have the feature yet. I think symmetrize, transpose, and triangle counting are related problems and can be addressed together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The common part of these 3 specialization could be factored out in a template function; e.g.,
Then in each specialization initialize the scrambles, masks, and the appropriate
brev
lambda; and pass all togenerate_v(...)
. Something like that. That way the underlying logic is factored out and only moving parts are kept separate.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And
brev
can be constructed in-place as a__device__
generic lambda (if possible... I think there might be some issues withnvcc
compiling generic device lambdas). But, if possible, something like:Obviously, different specializations of
scramble(...)
would instantiate different versions ofbrev
, accordingly, etc.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, thanks for pointing this out, and I refactored the code to remove redundancy.