Skip to content

CometoidWMTQualityEstimation processor implementation #151

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

Merged
merged 4 commits into from
Aug 5, 2025

Conversation

ssh-meister
Copy link
Collaborator

A processor for estimating translation quality using pretrained COMET-like models based on MarianNMT and the pymarian Evaluator

Copy link
Collaborator

@lilithgrigoryan lilithgrigoryan 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 the PR. Please see minor comments:)

self.model = None

def load_model(self):
from pymarian import Evaluator
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let’s wrap this in a try/except block. Attempt to import Evaluator, and if the import fails, log a message indicating that pymarian needs to be installed.


self.model = Evaluator(marian_args)

def process_dataset_entry(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there a reason to inherit from BaseParallel processor if we do not use process_dataset_entry

mock_processor.finalize = MagicMock()
mock_processor.number_of_entries = 0

# 👇 Patch load_model to avoid real downloading
Copy link
Collaborator

Choose a reason for hiding this comment

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

I love the emoji 😄 But sadly, our codebase is an emoji-free zone 🚫. Mind removing it?

@ssh-meister ssh-meister merged commit 3fce946 into main Aug 5, 2025
10 checks passed
@ssh-meister ssh-meister deleted the CometoidWMTQualityEstimation branch August 5, 2025 11:15
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