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

Possible error in sample_last_hop() function #11

Closed
alanjohnvarghese opened this issue Jan 19, 2023 · 1 comment
Closed

Possible error in sample_last_hop() function #11

alanjohnvarghese opened this issue Jan 19, 2023 · 1 comment

Comments

@alanjohnvarghese
Copy link
Contributor

Thank you for open sourcing the project, and for adding a lot of comments in it!

I think there is a logical error in line number 397 in utils.py : nnz = A[nnz, new_sample].nonzero()[1]. The issue is .nonzero() gives the non-zero indices with respect to the sub-matrix (A[nnz, new_sample]) and not with respect to the original matrix A.

This results in generating incorrect triplets. For example, for the following simple graph:

SimpleGraph

the triplet generated (with seed = 0) is:
0,1,3
1,0,2
2,3,4
3,2,0
4,3,2
3,2,4
3,0,4

Here the first column is the reference node, and the other columns are such that shortest_path(col0,col1) < shortest_path(col0, col2). The last triplet is incorrect because Node 4 is closer than Node 0 for Node 3. Changing line number 397 to: nnz = A[nodes, sampled].nonzero()[1] seems to fix the error.

@abojchevski
Copy link
Owner

Thanks for spotting this and for the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants