Skip to content

Conversation

@jbragg
Copy link
Contributor

@jbragg jbragg commented Aug 20, 2020

Currently it is impossible to use sacrebleu when len(predictions) != the number of references per prediction (very uncommon), due to a strange format expected by sacrebleu. If one passes in the data to nlp.metric.compute() in sacrebleu format, nlp throws an error due to mismatching lengths between predictions and references. If one uses a more standard format where predictions and references are lists of the same length, sacrebleu throws an error.

This PR transforms reference data in a more standard format into the unusual format expected by sacrebleu.

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting this !
Indeed sacrebleu expected this unusual format as input...

I think it would be better to check that all the references have the same length rather than cropping to the minimum length. If one length doesn't match the others, we should probably raise an error. What do you think ?

@jbragg
Copy link
Contributor Author

jbragg commented Aug 20, 2020

I think I agree @lhoestq so I pushed a change.
Thanks for your work on the library!

Copy link
Member

@thomwolf thomwolf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool, thanks @jbragg

Copy link
Member

@lhoestq lhoestq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice thank you !

@lhoestq lhoestq merged commit f33d598 into huggingface:master Aug 20, 2020
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.

3 participants