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

Fix Bleu Score smoothing function from taking log(0) #2839

Merged
merged 6 commits into from Oct 14, 2021

Conversation

robbyhorvath
Copy link
Contributor

@robbyhorvath robbyhorvath commented Oct 3, 2021

This PR pertains to the issue #2838. The issues problem was already fixed on the develop branch but created a new problem. The fix allowed the function to return 0 and then the logs were taken of the returned list values. I fixed it just by checking if the list elements are greater than 0.

@tomaarsen
Copy link
Member

@tomaarsen tomaarsen commented Oct 4, 2021

Could you perhaps add a test or tests that used to fail, and now pass? That would be helpful in preventing this issue from reappearing.

These might be places where such a test fits, though perhaps there's another spot where it fits:

@robbyhorvath
Copy link
Contributor Author

@robbyhorvath robbyhorvath commented Oct 6, 2021

Added a test to test_bleu.py. The issue was occurring with Smoothing method 4 when the hypothesis list length was 1

@stevenbird
Copy link
Member

@stevenbird stevenbird commented Oct 9, 2021

The result, few items in the tuple, is not an issue as the items just get summed (not, say, zipped with another tuple)

@stevenbird stevenbird self-assigned this Oct 12, 2021
@stevenbird stevenbird merged commit c692b0d into nltk:develop Oct 14, 2021
1 check passed
@stevenbird
Copy link
Member

@stevenbird stevenbird commented Oct 14, 2021

Thanks @robbyhorvath, @tomaarsen

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

Successfully merging this pull request may close these issues.

3 participants