cugraph icon indicating copy to clipboard operation
cugraph copied to clipboard

Add `is_multigraph` to PG and change `has_duplicate_edges` to use types

Open eriknw opened this issue 2 years ago • 2 comments

Closes #2591

Also, change default graph type to MultiGraph. allow_multi_edges keyword was renamed to do_expensive_check, which only occurs when the output graph is not a MultiGraph. Should do_expensive_check default to True or False?

eriknw avatar Sep 20 '22 03:09 eriknw

Codecov Report

Base: 59.75% // Head: 59.74% // Decreases project coverage by -0.01% :warning:

Coverage data is based on head (ec90585) compared to base (f95ee18). Patch coverage: 53.57% of modified lines in pull request are covered.

Additional details and impacted files
@@               Coverage Diff                @@
##           branch-22.10    #2708      +/-   ##
================================================
- Coverage         59.75%   59.74%   -0.02%     
================================================
  Files               111      111              
  Lines              6418     6436      +18     
================================================
+ Hits               3835     3845      +10     
- Misses             2583     2591       +8     
Impacted Files Coverage Δ
python/cugraph/cugraph/gnn/graph_store.py 68.83% <0.00%> (ø)
...h/cugraph/gnn/pyg_extensions/data/cugraph_store.py 81.49% <ø> (ø)
...ugraph/cugraph/dask/structure/mg_property_graph.py 13.36% <41.66%> (+0.59%) :arrow_up:
python/cugraph/cugraph/structure/property_graph.py 95.84% <66.66%> (-0.94%) :arrow_down:
python/cugraph/cugraph/structure/graph_classes.py 80.00% <0.00%> (+1.02%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar Sep 20 '22 05:09 codecov-commenter

Renamed do_expensive_check to check_multi_edges.

I just noticed allow_multi_edges is still used in cugraph_service. Should I take a look at this and try to figure out what to do?

eriknw avatar Sep 22 '22 17:09 eriknw

@alexbarghi-nv can you comment on the test failure here?

It is failing on cugraph/tests/test_pyg_extensions.py:test_get_subgraph

The edge data from the PropertyGraph is:

   _EDGE_ID_  _SRC_  _DST_    _TYPE_
0          0      3      1       cow
1          1      3      2       cow
2          2      1      4      duck
3          3      2      3      duck
4          4      2      4      duck
5          5      0      1     horse
6          6      0      2     horse
7          7      2      3  mongoose
8          8      1      4  mongoose
9          9      4      3     snake

and

   _EDGE_ID_  _SRC_  _DST_ _TYPE_
0          0      1      4    cat
1          1      2      3    cat
2          2      2      4    cat
3          3      0      2    dog
4          4      3      2    dog
5          5      4      3    dog
6          6      0      1    pig
7          7      2      3    pig
8          8      3      1    pig
9          9      1      4    pig

The test is dropping duplicates based only on SRC and DST columns, but not on TYPE. I don't know what the test is supposed to be testing; what is correct?

eriknw avatar Oct 01 '22 04:10 eriknw

rerun tests

BradReesWork avatar Oct 01 '22 20:10 BradReesWork

rerun tests

BradReesWork avatar Oct 03 '22 16:10 BradReesWork

pyg tests updated based on feedback from @alexbarghi-nv 🤞

eriknw avatar Oct 03 '22 22:10 eriknw

@alexbarghi-nv I fixed test_pyg_extensions.py:test_get_subgraph, but now test_pyg_extensions.py:test_neighbor_sample_multi_vertex is failing.

In this test, for cugraph_edge_type, the data from pG.get_edge_data is:

   _EDGE_ID_  _SRC_  _DST_ _TYPE_ edge_type
0          0      3      1    cow       cow
1          1      3      2    cow       cow

but data from row_dict for "cow" is

"black__cow__brown": [0]

eriknw avatar Oct 04 '22 03:10 eriknw

This issue didn't show up for me. Investigating...

alexbarghi-nv avatar Oct 04 '22 03:10 alexbarghi-nv

Thanks all for the reviews and help. Now we're waiting on CI...

eriknw avatar Oct 04 '22 21:10 eriknw

The fix to sampling_utils_impl.cuh in this PR appears to also fix https://github.com/rapidsai/cugraph/issues/2770

eriknw avatar Oct 05 '22 03:10 eriknw

@gpucibot merge

rlratzel avatar Oct 05 '22 15:10 rlratzel

@gpucibot merge

VibhuJawa avatar Oct 05 '22 15:10 VibhuJawa