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

Fix bug Random Walk in array sizes #2089

Merged

Conversation

ChuckHastings
Copy link
Collaborator

Random walk implementation returns a list of paths (vertex ids) and a list of edge weights for the edges on those paths.

@betochimas discovered that the size of the returned arrays is the same, even though the contents are different sizes. In his example, if the return path contains one path: [ 0, 1, 3, 5 ] it returns a weights array with 4 elements [0.1, 2.1, 7.2, ???] even though only the first three elements are valid. The weights are edge weights, the path [0,1,3,5] only has 3 edges. The first 3 elements in the returned array are correct. Interpreting the results as intended will never reference that value. However the sizes should be correct.

This PR fixes the initialization of the arrays to the correct size, which seems to correct the bug.

@ChuckHastings ChuckHastings requested a review from a team as a code owner February 22, 2022 19:50
@ChuckHastings ChuckHastings self-assigned this Feb 22, 2022
@ChuckHastings ChuckHastings added 3 - Ready for Review bug Something isn't working non-breaking Non-breaking change labels Feb 22, 2022
@ChuckHastings ChuckHastings added this to the 22.04 milestone Feb 22, 2022
Copy link
Contributor

@betochimas betochimas left a comment

Choose a reason for hiding this comment

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

Looks good to me!

betochimas added a commit to betochimas/cugraph that referenced this pull request Feb 22, 2022
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2022

Codecov Report

Merging #2089 (f8d9282) into branch-22.04 (df49ad7) will increase coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@               Coverage Diff                @@
##           branch-22.04    #2089      +/-   ##
================================================
+ Coverage         73.59%   73.64%   +0.04%     
================================================
  Files               156      156              
  Lines             10332    10332              
================================================
+ Hits               7604     7609       +5     
+ Misses             2728     2723       -5     
Impacted Files Coverage Δ
.../cugraph/tests/test_edge_betweenness_centrality.py 84.93% <0.00%> (+0.60%) ⬆️
...graph/cugraph/tests/test_betweenness_centrality.py 83.01% <0.00%> (+0.62%) ⬆️
python/cugraph/cugraph/tests/test_graph_store.py 100.00% <0.00%> (+1.56%) ⬆️
python/cugraph/cugraph/tests/test_utils.py 75.55% <0.00%> (+4.44%) ⬆️

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 df49ad7...f8d9282. Read the comment docs.

@ChuckHastings ChuckHastings changed the title fix bug in array sizes Fix bug Random Walk in array sizes Feb 22, 2022
@BradReesWork
Copy link
Member

@gpucibot merge

@rapids-bot rapids-bot bot merged commit a37bb06 into rapidsai:branch-22.04 Feb 24, 2022
@ChuckHastings ChuckHastings deleted the fix_random_walks_sizes branch August 4, 2022 18:26
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.

5 participants