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.

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

Nonstandard Beam Search (?) #16

Closed
ruiqi-zhong opened this issue Jul 30, 2021 · 2 comments
Closed

Nonstandard Beam Search (?) #16

ruiqi-zhong opened this issue Jul 30, 2021 · 2 comments

Comments

@ruiqi-zhong
Copy link

It's super minor, but I noticed that in the beam search implementation

https://github.com/ElementAI/duorat/blob/6fba4c3f08d372465780dea0a2198a650dd407f4/duorat/utils/beam_search.py#L59

it is collecting all the finished hypothesis in to an array, and expanding the rest of the hypothesis and taking the top K - len(finished). As a result, the finished hypothesis never drops out the final returned results, and the last candidate is effectively found by greedy decoding towards the end.

I believe a more standard (and perhaps better) implementation is to still keep the finished hypothesis in the beam, expand the other hypothesis, and take the top-K hypothesis from the union of the finished hypothesis and the other candidates. In this way, sub-optimal already-finished hypothesis can be excluded.

I do not believe it would substantially change the results in the paper, though; but just to point it out. Or maybe both versions are legit, and it's just I am unaware of the other version before.

(Reference: Algo 1 in this paper)

@RaymondLi0
Copy link
Collaborator

In our implementation, we do not consider any length-normalization, so the score of a given hypothesis strictly decreases as you add tokens to it.
As a consequence, if a finished hypothesis is in the top-K at some point, it will stay in the top-K until the end of decoding.
However this would not be the case if the score was not strictly decreasing. Then I believe doing what you say would indeed be a better solution. Does that make sense?

@ruiqi-zhong
Copy link
Author

yeah that makes sense. Thanks!

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

No branches or pull requests

2 participants