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.

Sign up for GitHub

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

Jump to bottom

TreeLSTM additions #1133

Merged
merged 3 commits into from
Dec 19, 2017
Merged

TreeLSTM additions #1133

merged 3 commits into from
Dec 19, 2017

Conversation

akoehn
Copy link
Contributor

@akoehn akoehn commented Dec 14, 2017

  • make Nary- and Unidirectional treelstm non-abstract
  • add nodes in arbitrary order if needed
  • swig bindings
  • documentation (fairly minimal)

@akoehn
make {Nary, Unidirectional}TreeLSTMBuilder non-abstract again
2ef2468 
Commit bf29c18 added set_h_impl to
RNNBuilder as an abstract method and the TreeLStmBuilders became
abstract.  This was fixed for the BidirectionalTreeLSTMBuilder but not
the other two.  This commit moves the the stub throwing a runtime error
from BidirectionalTreeLSTMBuilder to the TreeLSTMBuilder super class.
@akoehn
TreeLSTMs: add nodes in arbitrary order; swig bindings
20e02b8 
Main feature:
TreeLSTMs had to be constructed with the nodes in the tree added by
their index.  This is not practical if the nodes are already
sequentially ordered with the child nodes not being first, e.g. when
working with dependency trees.  If set_num_elements is called, a fixed
set of node space is reserved and nodes can be added in any order as
long as children are added first.

The old behavior still works as usual.

Also:
 - Add documentation to treelstm C++ code
 - add swig bindings
 - add scala wrapper for uni- and bidirectional tree lstm
@akoehn
scala uni- and bidirectional tree lstm wrapper
1afb05a
@neubig neubig merged commit 8d5580c into clab:master Dec 19, 2017
@neubig
Copy link
Contributor

neubig commented Dec 19, 2017

Thanks!

@akoehn akoehn deleted the swig-treelstm branch December 19, 2017 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@akoehn @neubig