-
Notifications
You must be signed in to change notification settings - Fork 543
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
Fix SDXL, Retinanet and GPTJ accuracy checker #2094
Conversation
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.]+)%", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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.]+)"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let me change it
I think we need the fix for gptj,retinanet and stable diffusion as the accuracy are failing:
llama2 and mixtral are working fine: |
@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.]+)'.*", |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
This will catch such failures in future. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug-fix
@arjunsuresh @pgmpablo157321 to help merge.
The accuracy checker is failing due to wrong regex pattern.