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

Add new new metric: Comet #52

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from
Draft

Add new new metric: Comet #52

wants to merge 4 commits into from

Conversation

cjlovering
Copy link
Collaborator

@cjlovering cjlovering commented May 10, 2022

  • Added dependency
  • Added wiring for using it by default in a translation task
  • (We should discuss the above -- i.e. is it desired?)
  • Uses the same framework as SARI -- expects a doc_to_rawtext to be
    implemented

TODOs

  • Test on a translation task. (Tentatively works, but I'm having memories issues on my laptop running the associated model or else some other problem is coming up. I will look into it further.)
  • Determine if we want to run it by default on all translation tasks (as is setup right now)

* Added dependency
* Added wiring for using it by default in a translation task
* (We should discuss the above -- i.e. is it desired?)
* Uses the same framework as SARI -- exepcts a `doc_to_rawtext` to be
implemented
@StellaAthena StellaAthena linked an issue May 10, 2022 that may be closed by this pull request
@StellaAthena
Copy link
Collaborator

@rbawden, do you think we should use this as the default?

@rbawden
Copy link

rbawden commented May 11, 2022

I'm not really sure. BLEU is still the most used metric by far I'd say, so we should include results for it and it might be wise to keep it as default (despite all the criticisms). I would tentatively say COMET could be a secondary metric, but it could be worth checking with some other people to see what the consensus is.

@cjlovering
Copy link
Collaborator Author

Update: I think that computing COMET after the pipeline has been run would be cleaner. I will return to this in a few days.

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.

Implement COMET
3 participants