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

directed edge weights on undirected graphs #5

Closed
simoons95 opened this issue Nov 16, 2022 · 6 comments · Fixed by #7
Closed

directed edge weights on undirected graphs #5

simoons95 opened this issue Nov 16, 2022 · 6 comments · Fixed by #7

Comments

@simoons95
Copy link

Hello,

First of all, thank you for your paper and your code, it is a pleasure to work with it.

However, I have a question about the following line :

edge_att = (att + transpose(data.edge_index, att, nodesize, nodesize, coalesced=False)[1]) / 2

When I run it, this line does not seem to do anything more than edge_att = (att + att) / 2.
As a result, edge weights are different depending on the direction of the edge (0 to 1 != 1 to 0).
Have I missed anything?

@siqim
Copy link
Member

siqim commented Nov 16, 2022

Thanks a lot for spotting and reporting this issue. After checking the code quickly, I find this is indeed a bug caused by PR #3. These issues are caused by edge_index that are not properly sorted, and I will fix this soon. Thank you very much!

@simoons95
Copy link
Author

simoons95 commented Nov 16, 2022

Can it really happen that input indices are not sorted (reason of PR3), given they come from a dataloader?

@siqim
Copy link
Member

siqim commented Nov 16, 2022

If I remember correctly, I did PR3 because I found data.edge_index is not sorted for dataset mutag though it's from a dataloader, i.e., it gives something like [[0, 1, 0, 2, 0], [1, 0, 2, 0, 3]], where the src tensor should be [0, 0, 0, 1, 2] if it's sorted.

@siqim
Copy link
Member

siqim commented Nov 16, 2022

I just fixed this issue by PR #7, and now the code should work properly. Thanks again for spotting this issue, and feel free to let us know if you encounter any more issues!

@siqim siqim pinned this issue Nov 16, 2022
@simoons95
Copy link
Author

simoons95 commented Nov 17, 2022

I see you go to cpu but never come back to gpu, which may lead to some bugs.
Maybe the following function could help:

from torch_geometric.utils import sort_edge_index

def reorder_like(from_edge_index, to_edge_index, values):
    from_edge_index, values = sort_edge_index(from_edge_index, values)
    ranking_score = to_edge_index[0] * (to_edge_index.max()+1) + to_edge_index[1]
    ranking = ranking_score.argsort().argsort()
    if not (from_edge_index[:, ranking] == to_edge_index).all():
        raise ValueError("Edges in from_edge_index and to_edge_index are different, impossible to match both.")
    return values[ranking]

You can use it like this:

                trans_idx, trans_val = transpose(data.edge_index, att, None, None, coalesced=False)
                trans_val_perm = reorder_like(trans_idx, data.edge_index, trans_val)
                edge_att = (att + trans_val_perm) / 2

@siqim
Copy link
Member

siqim commented Nov 18, 2022

Thanks a lot for the suggestion! I created a new PR #8 and updated the code as you suggested. :)

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

Successfully merging a pull request may close this issue.

2 participants