Skip to content

Constituency parse probabilities bug - #153

Open
etsipidi wants to merge 1 commit into
yzhangcs:mainfrom
etsipidi:pull_req
Open

Constituency parse probabilities bug#153
etsipidi wants to merge 1 commit into
yzhangcs:mainfrom
etsipidi:pull_req

Conversation

@etsipidi

@etsipidi etsipidi commented Sep 4, 2026

Copy link
Copy Markdown

Thank for this library! I'm using the constituency parser and its probabilities, and I noticed that while the parse trees are correct, there seems to be a bug in the returned probabilities. The probabilities matrices are missing a necessary row and column each (the column being the rightmost column, which holds the 'S' start symbol) and because of that, the matrices are currently not matching the parse trees.

The issue is in this line:

batch.probs = [prob[:i-1, 1:i].cpu() for i, prob in zip(lens, s_span)]

and I'm proposing this fix:

            batch.probs = [prob[:i, 1:i+1].cpu() for i, prob in zip(lens, s_span)]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant