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

Refactor k-core #2731

Merged
merged 102 commits into from
Nov 29, 2022
Merged

Refactor k-core #2731

merged 102 commits into from
Nov 29, 2022

Conversation

jnke2016
Copy link
Contributor

@jnke2016 jnke2016 commented Sep 24, 2022

This PR refactors k-core by leveraging the CAPI and updates the python cugraph tests. An MG implementation of k-core is also included in this PR

closes #2689
closes #2634
closes #2637
closes #2638

@jnke2016 jnke2016 requested review from a team as code owners September 24, 2022 22:36
@BradReesWork BradReesWork added this to the 22.10 milestone Sep 29, 2022
@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Sep 29, 2022
@@ -95,6 +95,13 @@ def ego_graph(G, n, radius=1, center=True, undirected=None, distance=None):

result_graph = type(G)(directed=G.is_directed())

if not G.edgelist.weights:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ egonet does not require weights. Not sure we should require that in python.

Copy link
Contributor Author

@jnke2016 jnke2016 Nov 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get a segmentation fault when running the CAPI tests without weights

python/cugraph/cugraph/community/subgraph_extraction.py Outdated Show resolved Hide resolved
@@ -110,6 +111,15 @@ def ego_graph(input_graph, n, radius=1, center=True):
# Initialize dask client
client = input_graph._client

# FIXME: Implement a better way to check if the graph is weighted similar
# to 'simpleGraph'
if len(input_graph.edgelist.edgelist_df.columns) != 3:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C++ egonet does not require weights. Not sure we should require that in python.

Copy link
Contributor Author

@jnke2016 jnke2016 Nov 23, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get a segmentation fault when running the CAPI tests without weights

@ChuckHastings ChuckHastings added 3 - Ready for Review and removed Blocked Cannot progress due to external reasons labels Nov 28, 2022
Copy link
Collaborator

@ChuckHastings ChuckHastings left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for removing the warnings.

python/cugraph/cugraph/community/egonet.py Outdated Show resolved Hide resolved
python/cugraph/cugraph/dask/community/egonet.py Outdated Show resolved Hide resolved
@@ -146,13 +146,16 @@ def jaccard(input_graph, vertex_pair=None, use_weight=False):

if use_weight:
raise ValueError(
"'use_weight' is currently not supported and must be set to 'False'"
"'use_weight' is currently not supported but will " "be in the next release"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would just say it's not supported and remove the "in the next release" part. I'm thinking schedule-related messaging should go in a blog or release note type of document.

python/pylibcugraph/pylibcugraph/k_core.pyx Outdated Show resolved Hide resolved
python/pylibcugraph/pylibcugraph/k_core.pyx Outdated Show resolved Hide resolved
python/pylibcugraph/pylibcugraph/k_core.pyx Outdated Show resolved Hide resolved
array containing the vertex identifiers, the first and second items are device
arrays containing respectively the hubs and authorities values for the corresponding
vertices
A tuple of device arrays containing the sources and destinations with
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vertex pairs are not necessarily source & destinations.

@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 2eed6eb into rapidsai:branch-22.12 Nov 29, 2022
@rlratzel rlratzel mentioned this pull request Dec 2, 2022
4 tasks
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
None yet
7 participants