harvardnlp / pytorch-struct

Fast, general, and tested differentiable structured prediction in PyTorch
http://harvardnlp.github.io/pytorch-struct
MIT License
1.11k stars 93 forks source link

Multi-root NonProjectiveDependencyCRF #86

Closed chijames closed 3 years ago

chijames commented 3 years ago

Hi,

Does NonProjectiveDependencyCRF support multi-root trees? If I understand correctly, [KGCPerezC07] did propose a method for the multi-root non-projective case.

Thanks!

srush commented 3 years ago

It looks like I only implemented single root. But the code is quite simple to change, and would be happy for a multi-root PR. It would be easy to test too because we have a way of generating all the multi-root, non-proj trees.

https://github.com/harvardnlp/pytorch-struct/blob/master/torch_struct/deptree.py#L213

chijames commented 3 years ago

Thanks for the reply! I am trying to implement the multi-root case.

Just curious, why is the current non-projective tree implemented as plain functions? (as opposed to class of projective single/multi root tree) I can definitely keep it like this and add a new argument for the deptree_nonproj function if you think that's better.