pytorch-tree-lstm
pytorch-tree-lstm copied to clipboard
unbounce
Implicit assumptions in code?
Commit 66f29a44e98c7332661b57d22501107bcb193f90 modified TreeLSTM class, specifically, regarding h_sum.
By using torch.unique_consecutive to construct h_sum on the fly seems to assume that if two edges have the same parent, they must be adjacent to each other in the list. Otherwise, when performing the following operation:
iou = self.W_iou(x) + self.U_iou(h_sum)
x and h_sum will have different 0th dimensions.
Yes, you are correct; I changed this recently as a performance optimization but I failed to update the documentation. The adjacency_list in the current code needs to have all parent nodes adjacent in the list. I will update the documentation to reflect this.
I'm reopening this just to help me keep track of what I still have to do :)
It can be forced inside the forward function of TreeLSTM layer.
edge_ordered_by_parent_indexes = adjacency_list[:, 0].argsort()
adjacency_list = adjacency_list[edge_ordered_by_parent_indexes]
edge_order = edge_order[edge_ordered_by_parent_indexes]
So that it doesn't matter if the adjacency matrix is sorted by parent_index or not After all, this constraint is there because of the way batched computations is done (torch.unique_consecutive).
