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
node2vec
wrapper to cugraph #2093Add
node2vec
wrapper to cugraph #2093Changes from 17 commits
a8c6634
70bccbd
226a2bc
9399259
010f87a
6b0b017
abe6eed
7184312
0495fc8
97f90b4
0b1886a
16ecf4a
f7cc0bb
e1a595c
8d6aad4
53d309a
465c6b3
494bdba
a640a54
e035788
19b06ce
44864e8
363ebbd
b06ccda
621dc95
154816c
bf1e221
9100f77
308984c
dacbba8
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.
follow up with function declaration I commented. I think
max_depth
should not be optional and must be a positive integer as you mentioned. I am not sure how the C++ part handles it if no value is passed. If this is updated, please could you update RW too as well as the test which I believe ensuremax_depth
is not None?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 assume you mean the description in the docstring right? I think I did that because the default value is None, but as you mentioned,
max_depth
certainly isn't optional, so I'll go ahead and remove that from node2vec and random_walksThere 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.
This call will return the source column of the renumbered dataframe and will raise an error if the user didn't create a cugraph.Graph from a renumbered dataframe. You should probably first check if G is renumbered to avoid this
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.
Investigating this right now, the pylibcugraph wrapper does not work as intended if renumbered is False and returns an incorrect input. I added a fixme on line 124 describing this, and I believe this has to be resolved from the pylibcugraph or c layer