Closed balvisio closed 1 year ago
Hi @mlabonne :
I just added another small fix in a new commit. I noticed that in the fit() function when using the NeighborLoader
the total_loss
was being divided by len(loader.dataset)
which in the case of the NeighborLoader
returns the total number of nodes since it is an instance of a NodeLoader
. IIUC we really want to divide by the total number of graphs so it is more appropriate to divide by len(loader.data)
. (Dividing by len(loader.dataset)
is appropriate when a DataLoader
was used)
Thanks!
Sorry about the delay. Thank you for your very valid comments, I'm merging your commits. Have a great day!
Hello @mlabonne , Thanks for the quick reply. I found another minor issue. According to the Pytorch documentation
CrossEntropyLoss
expects the logits so theforward()
method shouldn't applylog_softmax()
.Thanks!