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 SDXL, Retinanet and GPTJ accuracy checker #2094

Merged
merged 2 commits into from
Feb 6, 2025

Conversation

nvzhihanj
Copy link
Contributor

@nvzhihanj nvzhihanj commented Feb 6, 2025

@arjunsuresh @pgmpablo157321 to help merge.
The accuracy checker is failing due to wrong regex pattern.

@nvzhihanj nvzhihanj requested a review from a team as a code owner February 6, 2025 08:03
Copy link
Contributor

github-actions bot commented Feb 6, 2025

MLCommons CLA bot All contributors have signed the MLCommons CLA ✍️ ✅

@@ -735,19 +735,19 @@
ACC_PATTERN = {
"acc": r"^(?:\{\"accuracy|accuracy)[\": ]*=?\s*([\d\.]+).*",
"AUC": r"^AUC=([\d\.]+).*",
"mAP": r".*'(?:mAP|Total)':\s*([\d\.]+)",
"mAP": r".*(?:mAP|Total)=([\d.]+)%",
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

shall we do "mAP": r".*(?:mAP=|Total:)\s*([\d.]+)" ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me change it

@arjunsuresh
Copy link
Contributor

@anandhu-eng we need to fail the tests in MLC when the accuracy value is empty.

"ROUGE1": r".*'rouge1':\s([\d.]+).*",
"ROUGE2": r".*'rouge2':\s([\d.]+).*",
"ROUGEL": r".*'rougeL':\s([\d.]+).*",
"ROUGE1": r".*'rouge1':\s+'([\d.]+)'.*",
Copy link
Contributor

Choose a reason for hiding this comment

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

shall we do "ROUGE1": r".*'rouge1':\s+'?([\d.]+)'?.*", to make it work for both gptj and other LLMs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching, I wasn't testing llama

@arjunsuresh
Copy link
Contributor

This will catch such failures in future.

Copy link
Contributor

@arjunsuresh arjunsuresh left a comment

Choose a reason for hiding this comment

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

Bug-fix

@arjunsuresh arjunsuresh merged commit 2a41918 into mlcommons:master Feb 6, 2025
18 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Feb 6, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants