A bug in aug_random_edge

[skydetailme]

Hi, Shen. Thanks for your efforts in this program. I noticed there might have a logical bug in the aug_random_edge function.

“for i in index_list:
single_index_list.append(i)
index_list.remove((i[1], i[0]))”

Removing items in a list while iterating that list may cause problems, as the list changed but the current index is not changed.

This blog illustrated well.
https://thispointer.com/python-remove-elements-from-a-list-while-iterating/

Thanks for your help.

[yyou1996]

Hi @skydetailme,

Thanks for your interest in our work. Would you mind pointing out which file you are referring to? I think in my implementation I did not use "remove()" function but directly use indexing to modify them, e.g.

GraphCL/semisupervised_TU/pre-training/tu_dataset.py

Line 224 in 88f2a15

edge_index = np.concatenate((edge_index[:, np.random.choice(edge_num, (edge_num - permute_num), replace=False)], idx_add), axis=1)

[skydetailme]

Hi, thanks for the reply. Here is the link.
https://github.com/Shen-Lab/GraphCL/blob/master/unsupervised_Cora_Citeseer/aug.py

[yyou1996]

@yongduosui, would you mind giving some comments on this? Thanks!

[yongduosui]

@skydetailme The aim of this code is to convert to convert bidirectional edges index to unidirectional index. I just debug my code and did't find any bug. It remove the other edge index pair successfully. You can print the list and check. Thanks.

[skydetailme]

@skydetailme The aim of this code is to convert to convert bidirectional edges index to unidirectional index. I just debug my code and did't find any bug. It remove the other edge index pair successfully. You can print the list and check. Thanks.

Would you mind take some minutes to read the link I posted, it explains well.
And you will find that after this iteration, index_list is not empty, those are the items accidentally jumped because of index error. Those items are not solved.
The result may right on some occasions but this code may not logically right.

[skydetailme]

@skydetailme The aim of this code is to convert to convert bidirectional edges index to unidirectional index. I just debug my code and did't find any bug. It remove the other edge index pair successfully. You can print the list and check. Thanks.

Also, thanks for your help.

[skydetailme]

@skydetailme The aim of this code is to convert to convert bidirectional edges index to unidirectional index. I just debug my code and did't find any bug. It remove the other edge index pair successfully. You can print the list and check. Thanks.

It seems that if the adj matrix is self-loop, this code may cause a wrong result, and the index_list is not empty.
If it's not a self-loop adj matrix, the result is right.

[yyou1996]

@skydetailme Thank you for your carefulness, and I see your point. The citation net datasets happen to be without self-loop. I will read the post and give some comments after finishing my work at hand.

[skydetailme]

@skydetailme Thank you for your carefulness, and I see your point. The citation net datasets happen to be without self-loop. I will read the post and give some comments after finishing my work at hand.

Thanks for your reply and contribution. This project helps a lot.

[yyou1996]

Hi @skydetailme,

Sorry for the late reply that I just survive a DDL. I make certain modifications according to the blog u share https://thispointer.com/python-remove-elements-from-a-list-while-iterating/. Thanks again for your carefulness.