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

Fixes bug in NumberMap preventing use of string vertex IDs for MG graphs #2688

Conversation

rlratzel
Copy link
Contributor

fixes #2686

Summary

  • Moved assignment of a variable that sets a dtype to a point after the mappings from vertex strings to integer IDs are applied in order to capture the correct dtype of the internal dataframes. This bug was not detected earlier since integer vertex IDs had matching dtypes before and after any initial mappings were applied, and there was no test coverage for MG string vertex IDs.
  • Added test that uses a graph containing string vertex IDs then runs pagerank using both SG and MG versions and ensures the results are identical. This new test was based on the code provided in issue How to read the graph edge if the node value is non-numerical #2686
  • Fixed several bugs in MG test code that prevented pytest from collecting tests when the entire mg suite was specified. This probably would have been uncovered sooner if the MNMG nightly tests were configured to run differently (they run individual test files instead of having pytest collect all tests from the mg directory).

…using it to use the incoming data type instead of the mapped type, which matters more when strings are passed in since they must be mapped to ints. Added new MG test that uses string vertex IDs. Cleaned up and fixed bugs in various MG test files that prevented pytest from discovering and running all tests in the "mg" test directory.
@rlratzel rlratzel added bug Something isn't working non-breaking Non-breaking change labels Sep 13, 2022
@rlratzel rlratzel added this to the 22.10 milestone Sep 13, 2022
@rlratzel rlratzel requested a review from a team as a code owner September 13, 2022 03:58
@rlratzel rlratzel self-assigned this Sep 13, 2022
@codecov-commenter
Copy link

codecov-commenter commented Sep 13, 2022

Codecov Report

❗ No coverage uploaded for pull request base (branch-22.10@4657c68). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@               Coverage Diff               @@
##             branch-22.10    #2688   +/-   ##
===============================================
  Coverage                ?   60.10%           
===============================================
  Files                   ?      112           
  Lines                   ?     6151           
  Branches                ?        0           
===============================================
  Hits                    ?     3697           
  Misses                  ?     2454           
  Partials                ?        0           

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

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

@BradReesWork
Copy link
Member

LGTM - would like to see single GPU dask testing

@rlratzel
Copy link
Contributor Author

LGTM - would like to see single GPU dask testing

Good idea, I filed #2714 for that.

@rlratzel
Copy link
Contributor Author

@gpucibot merge

@rapids-bot rapids-bot bot merged commit b9d1842 into rapidsai:branch-22.10 Sep 23, 2022
@rlratzel rlratzel deleted the branch-22.10-bugfix_mg_string_vertex_ids branch September 28, 2023 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

How to read the graph edge if the node value is non-numerical
5 participants