Spacy: get_lca_matrix is incorrect

[slavi]

@ramananbalakrishnan

In spacy version 2.0.11

s = u"He created a test for spacy"
doc = nlp(s)
doc.get_lca_matrix()[3][5]

Expected: 3
Actual: 1

This branch of the recursion is incorrect:

https://github.com/explosion/spaCy/blob/master/spacy/tokens/doc.pyx#L732

It assumes that if n1 and n2 are not adjacent, then LCA(n1, n2) = LCA(n1.head, n2.head).

This is generally not true: there are many counterexamples, the above one with 3 nodes in a chain is the simplest.

[ramananbalakrishnan]

Hi @slavi , thanks for catching that.

I had ported this section of the code during the transition from v1 to v2 (relevant PR https://github.com/explosion/spaCy/pull/1443)

Would you be able to make the required change and send a pull request?

[honnibal]

Thanks for the report, and yes -- a pull request would be much appreciated!

[alvaroabascar]

I reimplemented the function to solve this issue, PR here.

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.