[Bugfix] Add pred and choices parsing to fix the issue of score=0 for…#258
[Bugfix] Add pred and choices parsing to fix the issue of score=0 for…#258Yanguan619 wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the MMStar dataset configuration by adding a post-processor and introduces a new helper function to parse options from question text. Feedback indicates that the new extraction logic is fragile and redundant since the options are already available in structured columns. It is recommended to use the existing options dictionary to ensure evaluation reliability.
| + prompt | ||
| + AIS_CONTENT_TAG | ||
| ) | ||
| choices = build_choices(extract_options_from_question(line['question'])) |
There was a problem hiding this comment.
Using extract_options_from_question(line['question']) to build choices is unreliable and likely the cause of incorrect evaluations. If the regex fails to match the question text (which is highly probable given its restrictive nature and the fact that line['question'] may not even contain the options in that specific format), choices will be empty. This leads to can_infer failing and a resulting score of 0. You should use the options dictionary defined at line 82, which is derived from the structured columns of the dataset and is already used to build the prompt.
| choices = build_choices(extract_options_from_question(line['question'])) | |
| choices = build_choices(options) |
| def extract_options_from_question(question_text): | ||
| options = {} | ||
|
|
||
| # 查找"Options:"部分 | ||
| if "Options:" in question_text: | ||
| # 获取Options部分 | ||
| options_part = question_text.split("Options:")[1].strip() | ||
|
|
||
| # 使用正则表达式匹配所有A-Z的选项 | ||
| pattern = r"([A-Z]):\s*([^,]+(?:,\s*[^,]+)*?)(?=(?:,\s*[A-Z]:|$))" | ||
|
|
||
| matches = re.findall(pattern, options_part) | ||
| for letter, content in matches: | ||
| # 清理内容：移除末尾的句点（如果存在） | ||
| content = content.strip() | ||
| if content.endswith("."): | ||
| content = content[:-1] | ||
| options[letter] = content | ||
|
|
||
| return options |
There was a problem hiding this comment.
The extract_options_from_question function is quite fragile. The regex r"([A-Z]):\s*([^,]+(?:,\s*[^,]+)*?)(?=(?:,\s*[A-Z]:|$))" assumes a very specific format (e.g., A: content, B: content) which may not be consistent across all samples. Moreover, it relies on the presence of the "Options:" string. Since the options are already structured in the dataframe columns and extracted into the options dictionary at line 82, parsing them from the raw text is unnecessary and prone to errors. If the structured columns are reliable, this function should be removed.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a post-processing step for MMStar dataset evaluation and adds a utility function to extract options from question text using regular expressions. While the changes aim to improve answer extraction, several critical issues were identified: the use of last_option_postprocess is risky as it may incorrectly capture letters from the model's reasoning, and a regression was introduced where options already parsed from dataset columns are ignored. Additionally, the regex for option extraction is fragile, and feedback was provided regarding PEP 8 compliance for type hints and the use of non-English comments.
| mmstar_eval_cfg = dict( | ||
| evaluator=dict(type=MMStarEvaluator) | ||
| evaluator=dict(type=MMStarEvaluator), | ||
| pred_postprocessor=dict(type=last_option_postprocess, options="ABCD"), |
There was a problem hiding this comment.
Using last_option_postprocess is risky for multiple-choice evaluation, especially if the model provides reasoning (Chain-of-Thought). This function extracts the last occurrence of any character in the options string ("ABCD"). If the model's explanation mentions other options (e.g., "Option B is incorrect, so the answer is A"), this will incorrectly return 'B' as the prediction.
Consider using first_option_postprocess or a more specific regex-based postprocessor that targets a final answer pattern (e.g., ANSWER: [A-D]).
| mmstar_eval_cfg = dict( | ||
| evaluator=dict(type=MMStarEvaluator) | ||
| evaluator=dict(type=MMStarEvaluator), | ||
| pred_postprocessor=dict(type=last_option_postprocess, options="ABCD"), |
There was a problem hiding this comment.
Using last_option_postprocess is risky here. Although the prompt asks for the answer on the last line, any mention of other option letters in the reasoning or in a trailing sentence will cause this postprocessor to extract the wrong letter.
Since the prompt explicitly asks for the ANSWER: [LETTER] format, it would be much safer to use a postprocessor that specifically extracts the letter following that prefix.
| + prompt | ||
| + AIS_CONTENT_TAG | ||
| ) | ||
| choices = build_choices(extract_options_from_question(line['question'])) |
There was a problem hiding this comment.
This change introduces a regression. By calling extract_options_from_question directly, you are ignoring the options already extracted from the TSV columns (lines 77-81). If a dataset provides options in columns but does not include the "Options:" string in the question text, choices will be empty, leading to a score of 0.
You should prioritize the options from the columns and only fallback to parsing the question text if they are missing.
| choices = build_choices(extract_options_from_question(line['question'])) | |
| choices = build_choices(options if options else extract_options_from_question(line['question'])) |
| logger = AISLogger() | ||
|
|
||
|
|
||
| def extract_options_from_question(question_text:str): |
There was a problem hiding this comment.
The type hint for question_text is missing a space after the colon, which deviates from PEP 8 style guidelines.
| def extract_options_from_question(question_text:str): | |
| def extract_options_from_question(question_text: str): |
| options = {} | ||
| if "Options:" in question_text: | ||
| options_part = question_text.split("Options:")[1].strip() | ||
| pattern = r"([A-Z]):\s*([^,]+(?:,\s*[^,]+)*?)(?=(?:,\s*[A-Z]:|$))" |
There was a problem hiding this comment.
The regex pattern is fragile because it strictly relies on commas as separators between options. If the dataset uses newlines, periods, or just spaces (e.g., A: Option 1 B: Option 2), this pattern will fail to correctly extract individual options. Additionally, if an option contains a comma that isn't followed by an option letter, the non-greedy match might behave unexpectedly.
|
|
||
| matches = re.findall(pattern, options_part) | ||
| for letter, content in matches: | ||
| # 清理内容：移除末尾的句点（如果存在） |
There was a problem hiding this comment.
The comment is in Chinese, which is inconsistent with the rest of the codebase. Please use English for comments and documentation.
| # 清理内容：移除末尾的句点（如果存在） | |
| # Clean content: remove trailing period if it exists |
1 participant
… mmstar datasets
Thanks for your contribution; we appreciate it a lot. The following instructions will make your pull request healthier and help you get feedback more easily. If you do not understand some items, don't worry, just make the pull request and seek help from maintainers.
感谢您的贡献，我们非常重视。以下说明将使您的拉取请求更健康，更易于获得反馈。如果您不理解某些项目，请不要担心，只需提交拉取请求并从维护人员那里寻求帮助即可。
PR Type / PR类型
Related Issue | 关联 Issue
Fixes #254
🔍 Motivation / 变更动机
Please describe the motivation of this PR and the goal you want to achieve through this PR.
请描述您的拉取请求的动机和您希望通过此拉取请求实现的目标。
📝 Modification / 修改内容
Please briefly describe what modification is made in this PR.
请简要描述此拉取请求中进行的修改。
📐 Associated Test Results / 关联测试结果
Please provide links to the related test results, such as CI pipelines, test reports, etc.
请提供相关测试结果的链接，例如 CI 管道、测试报告等。
🌟 Use cases (Optional) / 使用案例（可选）
If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能，最好在此处列出一些用例并更新文档。
cmd:
[ais_bench] [WARNING] result['details'] :
{ "type": "GEN", "0": { "prompt": [ { "role": "HUMAN", "prompt": [ { "image_url": { "url": "file:///data/xxx/benchmark/ais_bench/benchmark/datasets/utils/../../../../ais_bench/datasets/mmstar/MMStar_images/0.jpg" }, "type": "image_url" }, { "text": "Question: Which option describe the object relationship in the image correctly?\nOptions: A: The suitcase is on the book., B: The suitcase is beneath the cat., C: The suitcase is beneath the bed., D: The suitcase is beneath the book.\n", "type": "text" } ] } ], "origin_prediction": "The correct option is: **A: The suitcase is on the book.**\n\n### Explanation:\nLooking at the image:\n- There is a **brown suitcase** sitting inside what appears to be the trunk or cargo area of a car.\n- On top of the suitcase, there are various **stickers**.\n- **Below** the suitcase, there is an **open magazine or book** (with a car on the cover) lying on the car’s interior floor or cargo surface.\n- Therefore, the **suitcase is positioned on top of the book**.\n\nThis matches option **A**.\n\nThe other options are incorrect:\n- **B**: There is no cat in the image.\n- **C**: There is no bed.\n- **D**: The suitcase is *on top of* the book, not beneath it.\n\n✅ Final Answer: **A: The suitcase is on the book.**", "predictions": [ "The correct option is: **A: The suitcase is on the book.**\n\n### Explanation:\nLooking at the image:\n- There is a **brown suitcase** sitting inside what appears to be the trunk or cargo area of a car.\n- On top of the suitcase, there are various **stickers**.\n- **Below** the suitcase, there is an **open magazine or book** (with a car on the cover) lying on the car’s interior floor or cargo surface.\n- Therefore, the **suitcase is positioned on top of the book**.\n\nThis matches option **A**.\n\nThe other options are incorrect:\n- **B**: There is no cat in the image.\n- **C**: There is no bed.\n- **D**: The suitcase is *on top of* the book, not beneath it.\n\n✅ Final Answer: **A: The suitcase is on the book.**" ], "references": [ { "answer": "A", "category": "coarse perception", "choices": "{}", "l2-category": null, "split": null } ], "correct": [ false ] } }Fix
[ais_bench] [WARNING] result['details'] :
{ "type": "GEN", "0": { "prompt": [ { "role": "HUMAN", "prompt": [ { "image_url": { "url": "file:///data/xxx/benchmark/ais_bench/benchmark/datasets/utils/../../../../ais_bench/datasets/mmstar/MMStar_images/0.jpg" }, "type": "image_url" }, { "text": "Question: Which option describe the object relationship in the image correctly?\nOptions: A: The suitcase is on the book., B: The suitcase is beneath the cat., C: The suitcase is beneath the bed., D: The suitcase is beneath the book.\n", "type": "text" } ] } ], "origin_prediction": "The correct option is: **A: The suitcase is on the book.**\n\n### Explanation:\nLooking at the image:\n- There is a **brown suitcase** sitting inside what appears to be the trunk or cargo area of a car.\n- On top of the suitcase, there are various **stickers**.\n- **Below** the suitcase, there is an **open magazine or book** (with a car on the cover) that is partially underneath the suitcase.\n- So, the suitcase is **resting on top of** the book/magazine.\n\nThis matches option **A**: *The suitcase is on the book.*\n\nThe other options are incorrect:\n- **B**: There is no cat in the image.\n- **C**: There is no bed.\n- **D**: The suitcase is *on top of* the book, not beneath it.\n\n✅ Final Answer: **A: The suitcase is on the book.**", "predictions": [ "A" ], "references": [ { "answer": "A", "category": "coarse perception", "choices": "{\"A\": \"The suitcase is on the book\", \"B\": \"The suitcase is beneath the cat\", \"C\": \"The suitcase is beneath the bed\", \"D\": \"The suitcase is beneath the book\"}", "l2-category": null, "split": null } ], "correct": [ true ] } }✅ Checklist / 检查列表
Before PR:
After PR:
👥 Collaboration Info / 协作信息
🌟 Useful CI Command / 实用的CI命令
/gemini review/gemini summary/gemini help/readthedocs build