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

Is this the implementation of transductive learning? #8

Open
9310gaurav opened this issue May 6, 2019 · 8 comments
Open

Is this the implementation of transductive learning? #8

9310gaurav opened this issue May 6, 2019 · 8 comments

Comments

@9310gaurav
Copy link

I want to confirm if the implementation is of inductive learning or transductive learning. From the code it looks like Variable adj_lists contain all the edges in the graph and therefore test edges are also used during training. However the results reported in the paper correspond to inductive learning. Please let me know if I am missing anything.

@ShengdingHu
Copy link

I also noticed this difference. In fact, in the tensorflow version of original paper, the train_adj and val_adj are different. So I think this implementation detail might be missed by the author of pytorch version .

@9310gaurav
Copy link
Author

You are right. Also for the default arguments, the aggregator's "gcn" argument is set to False while for encoder it is set to True. That means aggregator does not consider self loops while aggregating. That's why the accuracy numbers are almost the same as stated in paper (they should be higher if test edges are also used during training). I hope the author could fix the two bugs so that other people using the code are aware. Thanks.

@ShengdingHu
Copy link

Thank you for reminding me of the self loops

@HongxuChenUQ
Copy link

HongxuChenUQ commented Jun 3, 2019

@9310gaurav @ShengdingHu Thanks for your comments, and making me aware about this issue.
The following is my observations, if I was wrong, please correct me.

In encoders.py. The self_loop was combined when the 'gcn' flag is set to false, while when 'gcn' is set to true, the self_loop is not combined. Therefore, the author set 'gcn' of the aggregator to False, and 'gcn' of the encoder to True, so that the entire model did not consider the self_loop.

So I think the settings in cora.run is correct (the author did it on purpose). However, the author treat the flag 'gcn' in aggregator.py and encoder.py inconsistent and differently.

@9310gaurav
Copy link
Author

@HongxuChenUQ I don't think the author did this on purpose. As mentioned in the paper, there are two ways self-loops can be supported in this architecture. First way is the GCN way where you use the self loop during aggregation. Second way is where you use it in the encoder by concatenating the self features with the aggregated neighbour features and then multiply them with the weights. Setting gcn as False in encoder and true in the aggregator implies not using any of these ways. If you set gcn to True in the encoder as well (GCN way), you will get improved accuracy.

@HongxuChenUQ
Copy link

@9310gaurav Thanks Gaurav. It makes clear for me now.

@EtoDemerzel0427
Copy link

I think it is just a simple implementation of the algorithm, and the datasets the author uses here is different from what he used in the paper. The cora and Pumbed datasets are small benchmark datasets and have been used in Thomas Kipf's GCN paper. So by all means, it is transductive learning.

@guotong1988
Copy link

mark

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

5 participants