yzhangcs / crfpar

[ACL'20, IJCAI'20] Code for "Efficient Second-Order TreeCRF for Neural Dependency Parsing" and "Fast and Accurate Neural CRF Constituency Parsing".
https://www.aclweb.org/anthology/2020.acl-main.302
MIT License
76 stars 7 forks source link

Why [0] was omitted in fn isprojective #5

Closed xsthunder closed 4 years ago

xsthunder commented 4 years ago

Thanks for releasing the code.

I found [0] was omitted in
https://github.com/yzhangcs/crfpar/blob/8abb95f177e5cbf4d7ebc494bcaf9ca15af3e3da/parser/utils/fn.py#L31

However, in yzhangcs/parser, [0] is included in calculation as far as i concerned. Was it a intended behavior? Can you explain the difference between the two scene. Following are yzhangcs/parser references.

https://github.com/yzhangcs/parser/blob/d4559e521ca88a3997337465a3c6d2bce4165d11/supar/utils/transform.py#L259

https://parser.readthedocs.io/en/latest/_modules/supar/utils/transform.html#CoNLL.isprojective

https://github.com/yzhangcs/parser/blob/d4559e521ca88a3997337465a3c6d2bce4165d11/tests/test_transform.py#L43

Thanks in advance

yzhangcs commented 4 years ago

Yes, they are expected. Actually, the two function are identical. In parser, the root position are cut out before fed into the function for api constituency. https://github.com/yzhangcs/parser/blob/2acbe9a24d9f91097c01b5f473f9f78e467cde34/supar/models/dependency.py#L260 https://github.com/yzhangcs/parser/blob/2acbe9a24d9f91097c01b5f473f9f78e467cde34/supar/utils/transform.py#L272

xsthunder commented 4 years ago

It makes sense. Thank you for the quick reply.