Skip to content

Add get_num_vertices and get_num_edges methods to PropertyGraph.#2434

Merged
rapids-bot[bot] merged 11 commits intorapidsai:branch-22.08from
eriknw:num_nodes_edges_by_type
Jul 29, 2022
Merged

Add get_num_vertices and get_num_edges methods to PropertyGraph.#2434
rapids-bot[bot] merged 11 commits intorapidsai:branch-22.08from
eriknw:num_nodes_edges_by_type

Conversation

@eriknw
Copy link
Contributor

@eriknw eriknw commented Jul 20, 2022

Closes #2422. I'll add this to MGPG when we finalize the API and behavior.

@eriknw
Add get_num_vertices and get_num_edges methods to PropertyGraph.
04a813b 
Closes rapidsai#2422.  I'll add this to MGPG when we finalize the API and behavior.
@eriknw eriknw requested a review from a team as a code owner July 20, 2022 06:20
@eriknw
Copy link
Contributor Author

eriknw commented Jul 20, 2022

This keeps pG.num_vertices andpG.num_edges as properties, which I think is convenient, and adds pG.get_num_vertices(type) and pG.get_num_edges(type).

If type is None, should we return the same as e.g. pG.num_vertices, or could we use dataframe[self.type_col_name].value_counts() to return the number for all types.

@BradReesWork BradReesWork added improvement Improvement / enhancement to an existing function non-breaking Non-breaking change labels Jul 20, 2022
@BradReesWork BradReesWork added this to the 22.08 milestone Jul 20, 2022
BradReesWork
BradReesWork requested changes Jul 20, 2022
View reviewed changes
rlratzel
rlratzel requested changes Jul 20, 2022
View reviewed changes
Copy link
Contributor

@rlratzel rlratzel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@eriknw and I had a conversation offline and we decided that it would be better to keep the API surface area smaller and minimize confusion by getting rid of the num_vertices and num_edges python property attributes, and just use the new getters here. The new getters should also support the caching behavior the python property attributes had.

rlratzel
rlratzel reviewed Jul 20, 2022
View reviewed changes
alexbarghi-nv
alexbarghi-nv approved these changes Jul 22, 2022
View reviewed changes
Copy link
Member

@alexbarghi-nv alexbarghi-nv left a comment

Choose a reason for hiding this comment

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

👍

@eriknw
Update get_num_{vertices,edges} for single-GPU PropertyGraph:
52d1532 
- add `include_edge_data=True` keyword to `get_num_vertices`
- improved docstrings of `get_num_vertices` and `get_num_edges`
- change default type name to `""`
- remove `num_vertices` and `num_edges` properties
- use `series.value_counts` to compute counts of types (assuming types are low cardinality)
- add and update tests
- copy `pG.num_vertices_with_properties` tests from rapidsai#2419

MG is not quite finished yet.
@codecov-commenter
Copy link

codecov-commenter commented Jul 22, 2022 •
edited
Loading

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.08@d925926). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             branch-22.08    #2434   +/-   ##
===============================================
  Coverage                ?   61.21%           
===============================================
  Files                   ?      106           
  Lines                   ?     5492           
  Branches                ?        0           
===============================================
  Hits                    ?     3362           
  Misses                  ?     2130           
  Partials                ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d925926...cd36ed5. Read the comment docs.

rlratzel
rlratzel reviewed Jul 28, 2022
View reviewed changes
Copy link
Contributor

@rlratzel rlratzel left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks good, I had one suggestion which I was wondering about.

@BradReesWork
Copy link
Member

BradReesWork commented Jul 29, 2022

@gpucibot merge

@rapids-bot rapids-bot bot merged commit 6b62002 into rapidsai:branch-22.08 Jul 29, 2022
@eriknw eriknw mentioned this pull request Jul 29, 2022
Closed
nv-rliu pushed a commit to nv-rliu/cugraph-gnn that referenced this pull request Jul 15, 2025
@eriknw
Add get_num_vertices and get_num_edges methods to PropertyGraph. (#2434)
e225d90 
Closes #2422.  I'll add this to MGPG when we finalize the API and behavior.

Authors:
  - Erik Welch (https://github.com/eriknw)

Approvers:
  - Alex Barghi (https://github.com/alexbarghi-nv)
  - Brad Rees (https://github.com/BradReesWork)
  - Rick Ratzel (https://github.com/rlratzel)

URL: rapidsai/cugraph#2434
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rlratzel rlratzel rlratzel approved these changes

@BradReesWork BradReesWork BradReesWork approved these changes

@alexbarghi-nv alexbarghi-nv alexbarghi-nv approved these changes

Assignees

No one assigned

Labels

improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Milestone

22.08

Development

Successfully merging this pull request may close these issues.

[FEA] API to get num_nodes and num_edges per type from the PropertyGraph

5 participants

@eriknw @codecov-commenter @BradReesWork @rlratzel @alexbarghi-nv