Skip to content

support mathvision#264

Merged
SJTUyh merged 2 commits into
AISBench:masterfrom
wenba0:mathvision
May 7, 2026
Merged

support mathvision#264
SJTUyh merged 2 commits into
AISBench:masterfrom
wenba0:mathvision

Conversation

@wenba0

@wenba0 wenba0 commented Apr 27, 2026

Copy link
Copy Markdown
Collaborator

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类型

  • Feature(功能新增)
  • Bugfix(Bug 修复)
  • Docs(文档更新)
  • CI/CD(持续集成/持续部署)
  • Refactor(代码重构)
  • Perf(性能优化)
  • Dependency(依赖项更新)
  • Test-Cases(测试用例更新)
  • Other(其他)

🔍 Motivation / 变更动机

新增mathvision数据集

📝 Modification / 修改内容

新增mathvision数据集

📐 Associated Test Results / 关联测试结果

增加冒烟用例accuracy_mathvision
image

⚠️ BC-breaking (Optional) / 向后不兼容变更(可选)

Does the modification introduce changes that break the backward compatibility of the downstream repositories? If so, please describe how it breaks the compatibility and how the downstream projects should modify their code to keep compatibility with this PR.
是否引入了会破坏下游存储库向后兼容性的更改?如果是,请描述它如何破坏兼容性,以及下游项目应该如何修改其代码以保持与此 PR 的兼容性。

⚠️ Performance degradation (Optional) / 性能下降(可选)

If the modification introduces performance degradation, please describe the impact of the performance degradation and the expected performance improvement.
如果引入了性能下降,请描述性能下降的影响和预期的性能改进。

🌟 Use cases (Optional) / 使用案例(可选)

If this PR introduces a new feature, it is better to list some use cases here and update the documentation.
如果此拉取请求引入了新功能,最好在此处列出一些用例并更新文档。

✅ Checklist / 检查列表

Before PR:

  • Pre-commit or other linting tools are used to fix the potential lint issues. / 使用预提交或其他 linting 工具来修复潜在的 lint 问题。
  • Bug fixes are fully covered by unit tests, the case that causes the bug should be added in the unit tests. / 修复的 Bug 已完全由单元测试覆盖,导致 Bug 的情况应在单元测试中添加。
  • The modification is covered by complete unit tests. If not, please add more unit tests to ensure the correctness. / 此拉取请求中的修改已完全由单元测试覆盖。如果不是,请添加更多单元测试以确保正确性。
  • All relevant documentation (API docs, docstrings, example tutorials) has been updated to reflect these changes. / 所有相关文档(API 文档、文档字符串、示例教程)已更新以反映这些更改。

After PR:

  • If the modification has potential influence on downstream or other related projects, this PR should be tested with those projects. / 如果此拉取请求对下游或其他相关项目有潜在影响,应在那些项目中测试此 PR。
  • CLA has been signed and all committers have signed the CLA in this PR. / CLA 已签署,且本 PR 中的所有提交者均已签署 CLA。

👥 Collaboration Info / 协作信息

  • Suggested Reviewers / 建议审核人: @xxx
  • Relevant Module Owners / 相关模块负责人: @xxx
  • Other Collaboration Notes / 其他协作说明:

🌟 Useful CI Command / 实用的CI命令

Command / 命令 Introduction / 介绍
/gemini review Performs a code review for the current pull request in its current state by Gemini. / 对当前拉取请求在当前状态下由 Gemini 执行代码审核。
/gemini summary Provides a summary of the current pull request in its current state by Gemini. / 对当前拉取请求在当前状态下由 Gemini 提供摘要。
/gemini help Displays a list of available commands of Gemini. / 显示 Gemini 可用命令的列表。
/readthedocs build Triggers a build of the documentation for the current pull request in its current state by Read the Docs. / 触发当前拉取请求在当前状态下由 Read the Docs 构建文档。

@wenba0 wenba0 temporarily deployed to smoke-test-approval April 27, 2026 03:44 — with GitHub Actions Inactive

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the MathVision (MATH-V) dataset for multimodal mathematical reasoning, providing documentation, configuration files, and the core implementation for data loading and evaluation. The reviewer feedback suggests refining the multiple-choice answer normalization to prioritize 1-indexing, enhancing the robustness of image storage paths to support read-only environments, and standardizing the evaluation metric keys for consistency across the codebase.

Comment on lines +162 to +167
if upper_answer.isdigit():
index = int(upper_answer)
if 0 <= index < len(options):
return allowed_letters[index]
if 1 <= index <= len(options):
return allowed_letters[index - 1]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The logic for converting numeric answers to letters is ambiguous. Currently, if a model outputs "1", it returns 'B' (index 1) because the 0-indexed check comes first. However, in multiple-choice questions, numeric answers are more commonly 1-indexed (i.e., "1" maps to 'A'). Prioritizing 1-indexing would align better with common model behavior for MCQ tasks.

Suggested change
if upper_answer.isdigit():
index = int(upper_answer)
if 0 <= index < len(options):
return allowed_letters[index]
if 1 <= index <= len(options):
return allowed_letters[index - 1]
if upper_answer.isdigit():
index = int(upper_answer)
if 1 <= index <= len(options):
return allowed_letters[index - 1]
if 0 <= index < len(options):
return allowed_letters[index]

if is_local:
base_dir = resolved_path if os.path.isdir(resolved_path) else os.path.dirname(resolved_path)
else:
base_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), '../../..', 'datasets'))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The base_dir for storing images is calculated using a hardcoded relative path from the source file. This approach is fragile and will likely fail if the package is installed in a read-only location (e.g., as a system-wide site-package). It is recommended to use the get_cache_dir utility or respect the AIS_BENCH_DATASETS_CACHE environment variable to ensure a writable location is used for image extraction when the dataset is loaded from a remote source.

if subject:
subject_scores.setdefault(f'subject: {subject}', []).append(score)

result = {'Accuracy': 100 * sum(scores) / len(scores) if scores else 0.0}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The evaluation result uses the key 'Accuracy' (capitalized), whereas other evaluators in this repository (e.g., MATHEvaluator in math.py) consistently use 'accuracy' (lowercase). Using the lowercase key ensures consistency across the benchmark suite and compatibility with reporting tools that expect the standard key name.

Suggested change
result = {'Accuracy': 100 * sum(scores) / len(scores) if scores else 0.0}
result = {'accuracy': 100 * sum(scores) / len(scores) if scores else 0.0}

@wenba0 wenba0 temporarily deployed to smoke-test-approval May 7, 2026 01:37 — with GitHub Actions Inactive
@SJTUyh SJTUyh merged commit 2c06b6c into AISBench:master May 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants