williamleif / graphsage-simple

Simple reference implementation of GraphSAGE.
993 stars 242 forks source link

question about the GCN type aggregators #6

Open Liuxx21 opened 5 years ago

Liuxx21 commented 5 years ago

the code in run_cora(), model.py agg1 = MeanAggregator(features, cuda=True) enc1 = Encoder(features, 1433, 128, adj_lists, agg1, gcn=True, cuda=False) agg2 = MeanAggregator(lambda nodes : enc1(nodes).t(), cuda=False) enc2 = Encoder(lambda nodes : enc1(nodes).t(), enc1.embed_dim, 128, adj_lists, agg2, base_model=enc1, gcn=True, cuda=False) enc1's gcn flag is set to True, while the agg1's gcn flag is default False and never be set to True in enc1's code.

so the self-loop of GCN is not considered in fact.

I wonder if there might be something wrong with it, isn't it?

Liuxx21 commented 5 years ago

I think the code can be corrected by adding this: self.aggregator.gcn = gcn

Liuxx21 commented 5 years ago

and this is when i try to see the gcn flag of enc1 and agg1 image

chengzs123 commented 3 years ago

I think so too. After reading the paper carefully, the gcn-type aggregation method should include its own nodes. image

chengzs123 commented 3 years ago

image image As you said, the two GCNs do not correspond. When defining the agg aggregate function, I directly pass in gcn=True to get a higher F1 score, compared with gcn=False