Optimize the drop-duplicate functionality #4095

jnke2016 · 2024-01-16T22:02:06Z

Our current python API leverages dask to implement the drop-duplicate functionality but it carries a lot of overhead as it draws a significant amount of host memory and results into a crash when processing large graphs (4+ billion edges).

This PR

Leverages the CAPI to internally drop multi edges when creating the PLC graph.
Deprecates the parameter multi which, when set to False, triggers the dask based drop-duplicate functionality
Add flag do_expensive_check to check for NULL values in the edgelist

…2_enable-drop-duplicates

rlratzel · 2024-01-24T22:01:53Z

python/cugraph/cugraph/structure/symmetrize.py

+        warnings.warn(
+            "'multi' is deprecated as the removal of multi edges is "
+            "only supported when creating a multi-graph",
+            FutureWarning,
+        )


I think I'm missing something: does this mean a user who wants a MultiGraph could also want to remove the multiple edges? Why would they want a MultiGraph without multiple edges? It seems like this option would be more useful for a Graph, but it's being deprecated here.

I think I'm missing something: does this mean a user who wants a MultiGraph could also want to remove the multiple edges

No, It means that the only way to drop multi edges is by creating a multi-graph. In fact prior to that, we were using the flag multi in the symmetrize function to drop multi-edges (if creating a directed or undirected Graph) through the _memory_efficient_drop_duplicates function which is dask based. But we can't just get rid of that flag because some users might still have it in their code base if they were directly calling the symmetrize function directly

Okay, thanks. I think my concerns will go away when Vibhu's feedback is addressed.

VibhuJawa

Looks good mostly apart from some changes around docstrings.

Can we also quickly also run the graph creation benchmarks here too see how perf may change.

https://github.com/rapidsai/cugraph/blob/branch-24.02/benchmarks/cugraph/standalone/cugraph_graph_creation.py

VibhuJawa · 2024-01-25T00:42:40Z

python/cugraph/cugraph/structure/symmetrize.py

    multi : bool, optional (default=False)
+        Deprecated.
        Set to True if graph is a Multi(Di)Graph. This allows multiple
        edges instead of dropping them.


I would also mention the warning statement along with Deprecated, otherwise it is confusing for the user.

Suggested change

multi : bool, optional (default=False)

Deprecated.

Set to True if graph is a Multi(Di)Graph. This allows multiple

edges instead of dropping them.

multi : bool, optional (default=False)

[Deprecated, Multi will be removed in future version, and the removal of multi

edges will no longer be supported from 'symmetrize'. Multi edges will be removed

upon creation of graph instance directly based on if the graph is

`curgaph.MultiGraph` or `cugraph.Graph`. ]

Set to True if graph is a Multi(Di)Graph. This allows multiple

edges instead of dropping them.

Also will suggest filing an issue for tracking this deprecation.

VibhuJawa · 2024-01-25T00:42:58Z

python/cugraph/cugraph/structure/symmetrize.py

@@ -208,6 +224,7 @@ def symmetrize(
        weights column name.

    multi : bool, optional (default=False)
+        Deprecated.


Same comment here.

…2_enable-drop-duplicates

VibhuJawa

Mostly looks good, just some clarification around if we are dropping duplicates for single GPU's only.

VibhuJawa · 2024-01-31T17:59:25Z

python/cugraph/cugraph/structure/graph_implementation/simpleGraph.py

+            edgelist_df = edgelist_df.groupby(
+                by=[*vertex_col_name], as_index=False
+            ).min()


Just confirming, edgelist_df is a cuDF data frame right ?

Yes it is a cuDF dataframe

VibhuJawa · 2024-01-31T18:02:43Z

python/cugraph/cugraph/structure/graph_implementation/simpleGraph.py

+        if not self.properties.multi_edge:
+            # Drop parallel edges for non MultiGraph
+            # FIXME: Drop multi edges with the CAPI instead.
+            df = df.groupby(by=[*vertex_col_name], as_index=False).min()


Same question here , assuming this a cuDF dataframe.

Yes it is a cuDF dataframe

VibhuJawa · 2024-01-31T18:03:36Z

python/cugraph/cugraph/tests/data_store/test_property_graph.py

@@ -1176,10 +1176,12 @@ def test_extract_subgraph_vertex_prop_condition_only(
        )
    # Should result in two edges, one a "relationship", the other a "referral"
    expected_edgelist = cudf.DataFrame(
-        {"src": [89216, 78634], "dst": [78634, 89216], "weights": [99, 8]}
+        {"src": [78634, 89216], "dst": [89216, 78634], "weights": [8, 99]}


Wonder why we flipped it around ?

I initially removed the drop_duplicate call which was swapping the order of some columns. This expected result was set to match the actual_result. But this is no longer relevant since I am no longer updating this test file

Thanks for fixing this.

rlratzel

Approving assuming all of @VibhuJawa 's feedback gets addressed.

jnke2016 · 2024-02-01T14:59:47Z

just some clarification around if we are dropping duplicates for single GPU's only.

@VibhuJawa that's correct

VibhuJawa

LGTM

…2_enable-drop-duplicates

rlratzel · 2024-02-02T00:00:47Z

/merge

add SG support for dropping multi-edges through the CAPI

883644c

jnke2016 requested a review from a team as a code owner January 16, 2024 22:02

github-actions bot added the python label Jan 16, 2024

jnke2016 added 5 commits January 16, 2024 17:04

add MG support for dropping multi-edges and deprecaate parameter

6113946

fix style

f92d936

fix copyright

64ec680

Merge remote-tracking branch 'upstream/branch-24.02' into branch-24.0…

2e4b0a7

…2_enable-drop-duplicates

Merge remote-tracking branch 'upstream/branch-24.02' into branch-24.0…

0695fed

…2_enable-drop-duplicates

BradReesWork requested review from rlratzel and VibhuJawa January 23, 2024 19:19

BradReesWork added this to the 24.02 milestone Jan 23, 2024

jnke2016 added 6 commits January 24, 2024 11:14

fix typo

0442e53

fix typo

fd98039

reorder arguments

1d87370

Merge remote-tracking branch 'upstream/branch-24.02' into branch-24.0…

1d39ec6

…2_enable-drop-duplicates

add 'do_expensive_check'

c4d9580

fix style

ad62f33

rlratzel reviewed Jan 24, 2024

View reviewed changes

update graph creation warning description

779bd2d

VibhuJawa suggested changes Jan 25, 2024

View reviewed changes

jnke2016 added 10 commits January 26, 2024 08:52

update docstrings

5900bd8

fix style

705788f

drop duplicates when viewing the edgelist

5fa81c4

drop duplicates when viewing edges and update tests

c85f622

fix style

8f7436e

remove debug print

7c165e7

remove unused import

fc5e627

Merge remote-tracking branch 'upstream/branch-24.02' into branch-24.0…

50a11c0

…2_enable-drop-duplicates

undo changes to test

f3f1c3f

drop duplicate edges, update tests and copyright

77fe0f5

jnke2016 added 9 commits January 30, 2024 21:26

fix style

7db4a10

uncommment test

b3077cf

fix typo

bdcd215

remove outdated comment

2c6892a

remove debug print

ff5373a

update tests

1efef76

fix style

ebbd33f

update number of edges count

ed76013

fix style

890f885

VibhuJawa approved these changes Jan 31, 2024

View reviewed changes

rlratzel approved these changes Jan 31, 2024

View reviewed changes

jnke2016 added 4 commits February 1, 2024 04:31

reset changes to sg

ccc5189

update tests

63a669b

fix style

a6d767f

update copyright

e7ea077

VibhuJawa approved these changes Feb 1, 2024

View reviewed changes

Merge remote-tracking branch 'upstream/branch-24.02' into branch-24.0…

ad2602c

…2_enable-drop-duplicates

BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Feb 1, 2024

rapids-bot bot merged commit 3d52f17 into rapidsai:branch-24.02 Feb 2, 2024
109 checks passed

rlratzel mentioned this pull request Feb 5, 2024

[BUG]: Graph Creation Failure at Scale #4076

Closed

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Optimize the drop-duplicate functionality #4095

Optimize the drop-duplicate functionality #4095

jnke2016 commented Jan 16, 2024 •

edited

Loading

rlratzel Jan 24, 2024 •

edited

Loading

jnke2016 Jan 24, 2024 •

edited

Loading

rlratzel Jan 25, 2024

VibhuJawa left a comment

VibhuJawa Jan 25, 2024

VibhuJawa Jan 25, 2024

VibhuJawa Jan 25, 2024

VibhuJawa left a comment

VibhuJawa Jan 31, 2024

jnke2016 Feb 1, 2024

VibhuJawa Feb 1, 2024

VibhuJawa Jan 31, 2024

jnke2016 Feb 1, 2024

VibhuJawa Feb 1, 2024

VibhuJawa Jan 31, 2024

jnke2016 Feb 1, 2024

VibhuJawa Feb 1, 2024

rlratzel left a comment

jnke2016 commented Feb 1, 2024

VibhuJawa left a comment

rlratzel commented Feb 2, 2024

Optimize the drop-duplicate functionality #4095

Optimize the drop-duplicate functionality #4095

Conversation

jnke2016 commented Jan 16, 2024 • edited Loading

rlratzel Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

jnke2016 Jan 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VibhuJawa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

VibhuJawa left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rlratzel left a comment

Choose a reason for hiding this comment

jnke2016 commented Feb 1, 2024

VibhuJawa left a comment

Choose a reason for hiding this comment

rlratzel commented Feb 2, 2024

jnke2016 commented Jan 16, 2024 •

edited

Loading

rlratzel Jan 24, 2024 •

edited

Loading

jnke2016 Jan 24, 2024 •

edited

Loading