feat: implement transforms for summarization accuracy metrics #211
Conversation
| require( | ||
| set(input_keys) == {target_output_key, model_output_key}, | ||
| f"input_keys to MeteorScore should be {target_output_key, model_output_key}.", | ||
| ) |
There was a problem hiding this comment.
Question: Why are target_output_key and model_output_key being passed as additional parameters, if input_keys should already contain these?
Is there a way we can remove the need to pass in repeated parameters?
There was a problem hiding this comment.
Yeah, so the reason I added them as additional parameters is that we need to be able to distinguish between the two. We could rely on a convention where input_keys[0] is always interpreted to be the target output key for example, but we both know that relying on conventions that aren't explicitly enforced by the code is a recipe for disaster.
Right now, the Transform ABC implementation coupled with the way we create copies of transforms in execute pretty much forces all transforms to have input_keys and output_keys as their first two arguments. So even though in this case, we'd ideally just not pass input_keys, under the current structure, we have to.
I'd like to make a follow up PR where I change things so that this is no longer the case, as I agree that it results in ugly, redundant code.
Description of changes:
This PR implements
Transforms for the three existing summarization accuracy metrics supported byfmeval: Meteor, Rouge, and Bert.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.