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 confusing print statement in TranslationEvaluator #1894

Merged
merged 3 commits into from
Jun 4, 2024

Conversation

NathanS-Git
Copy link
Contributor

Target should not be the models guess, but the actual given target value. The models guesses are given right below.

This PR additionally fixes/removes some unnecessary python and logic.

@tomaarsen
Copy link
Collaborator

Hello!

Apologies for the delay. I think this is a great idea; I've updated the code to create this:

Incorrect  : Source 1361 is most similar to target 1360 instead of target 1361
Source     : Write your name in capitals.
Pred Target: Schrijf uw naam in hoofdletters. (Score: 0.8223)
True Target: Schrijf je naam in hoofdletters. (Score: 0.8159)
         1360 (Score: 0.8223) Schrijf uw naam in hoofdletters.
         1362 (Score: 0.8173) Schrijf jullie naam in hoofdletters.
         1361 (Score: 0.8159) Schrijf je naam in hoofdletters.
         1363 (Score: 0.4340) Ik weet wat jouw naam is.
         1068 (Score: 0.4309) Schrijft ge een brief?

Incorrect  : Source 1362 is most similar to target 1360 instead of target 1362
Source     : Write your name in capitals.
Pred Target: Schrijf uw naam in hoofdletters. (Score: 0.8223)
True Target: Schrijf jullie naam in hoofdletters. (Score: 0.8173)
         1360 (Score: 0.8223) Schrijf uw naam in hoofdletters.
         1362 (Score: 0.8173) Schrijf jullie naam in hoofdletters.
         1361 (Score: 0.8159) Schrijf je naam in hoofdletters.
         1363 (Score: 0.4340) Ik weet wat jouw naam is.
         1068 (Score: 0.4309) Schrijft ge een brief?

Which I think is fairly clear. Thanks for setting this up!

  • Tom Aarsen

@tomaarsen tomaarsen changed the title Fix confusing print statement Fix confusing print statement in TranslationEvaluator Jun 4, 2024
@tomaarsen tomaarsen merged commit 4f11200 into UKPLab:master Jun 4, 2024
9 checks passed
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.

2 participants