search

dasguptar / treelstm.pytorch

Tree LSTM implementation in PyTorch
MIT License
550 stars 139 forks source link

ChildSumTreeLSTM : fx and fh linear layer are declare but is not used #4

Closed ttpro1995 closed 7 years ago

ttpro1995 commented 7 years ago

Line 21, 22

self.fx = nn.Linear(self.in_dim,self.mem_dim)
self.fh = nn.Linear(self.mem_dim,self.mem_dim)

But it is never use

I think you intend to use in line 38, 39. (perhaps typo ix with fx )

fx = F.torch.unsqueeze(self.ix(inputs),1)
f = F.torch.cat([self.ih(child_hi)+fx for child_hi in child_h], 0)
dasguptar commented 7 years ago

Whoa! Good catch @ttpro1995 So it seems I was a complete klutz, and declared fx, fh but used ix, ih instead. Thanks for spotting it! Let me run the model with the fix and as soon as that is done I will merge your PR as well

dasguptar commented 7 years ago

@ttpro1995 Closing this issue for now, because I merged the PR. Let me know if you find any other bugs!