Skip to content

Conversation

@jiangyunfan1
Copy link
Contributor

@jiangyunfan1 jiangyunfan1 commented Nov 21, 2025

What this PR does / why we need it?

This PR updates the acc standard for deepseek mtpx cases, according to inner standard

Does this PR introduce any user-facing change?

No

How was this patch tested?

By running the test

@github-actions
Copy link

👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:‌‌

  • A PR should do only one thing, smaller PRs enable faster reviews.
  • Every PR should include unit tests and end-to-end tests ‌to ensure it works and is not broken by other future PRs.
  • Write the commit message by fulfilling the PR description to help reviewer and future developers understand.

If CI fails, you can run linting and testing checks locally according Contributing and Testing.

Copy link
Contributor

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

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 updates an end-to-end test by adding a new accuracy test case (gsm8k) and structuring the test configurations based on different modes (mtp2, mtp3). The changes are clear and improve the test setup. My feedback focuses on adhering to Python's PEP 8 style guide for naming conventions to improve code readability and maintainability.

Comment on lines 41 to 63
aisbench_gsm8k = [{
"case_type": "accuracy",
"dataset_path": "vllm-ascend/gsm8k-lite",
"request_conf": "vllm_api_general_chat",
"dataset_conf": "gsm8k/gsm8k_gen_0_shot_noncot_chat_prompt",
"max_out_len": 32768,
"batch_size": 32,
"baseline": 95,
"threshold": 5
}]

aisbench_aime = [{
"case_type": "accuracy",
"dataset_path": "vllm-ascend/aime2024",
"request_conf": "vllm_api_general_chat",
"dataset_conf": "aime2024/aime2024_gen_0_shot_chat_prompt",
"max_out_len": 32768,
"batch_size": 32,
"baseline": 80,
"baseline": 86.67,
"threshold": 7
}]

aisbench_case_dict = {"mtp2": aisbench_gsm8k, "mtp3": aisbench_aime}
Copy link
Contributor

Choose a reason for hiding this comment

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

high

According to PEP 8, module-level constants should be written in all capital letters with underscores separating words.1 Since aisbench_gsm8k, aisbench_aime, and aisbench_case_dict are used as constants, it's recommended to rename them to AISBENCH_GSM8K, AISBENCH_AIME, and AISBENCH_CASE_DICT respectively.

AISBENCH_GSM8K = [{
    "case_type": "accuracy",
    "dataset_path": "vllm-ascend/gsm8k-lite",
    "request_conf": "vllm_api_general_chat",
    "dataset_conf": "gsm8k/gsm8k_gen_0_shot_noncot_chat_prompt",
    "max_out_len": 32768,
    "batch_size": 32,
    "baseline": 95,
    "threshold": 5
}]

AISBENCH_AIME = [{
    "case_type": "accuracy",
    "dataset_path": "vllm-ascend/aime2024",
    "request_conf": "vllm_api_general_chat",
    "dataset_conf": "aime2024/aime2024_gen_0_shot_chat_prompt",
    "max_out_len": 32768,
    "batch_size": 32,
    "baseline": 86.67,
    "threshold": 7
}]

AISBENCH_CASE_DICT = {"mtp2": AISBENCH_GSM8K, "mtp3": AISBENCH_AIME}

Style Guide References

Footnotes

  1. PEP 8 recommends using uppercase for constants. 'Constants are usually defined on a module level and written in all capital letters with underscores separating words.'

assert choices[0].text, "empty response"
print(choices)
# aisbench test
aisbench_cases = aisbench_case_dict[mode]
Copy link
Contributor

Choose a reason for hiding this comment

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

high

To maintain consistency with the proposed renaming of aisbench_case_dict to AISBENCH_CASE_DICT (in adherence with PEP 8), this usage should also be updated.

Suggested change
aisbench_cases = aisbench_case_dict[mode]
aisbench_cases = AISBENCH_CASE_DICT[mode]

Signed-off-by: jiangyunfan1 <jiangyunfan1@h-partners.com>
@jiangyunfan1 jiangyunfan1 changed the title [TEST]update acc [TEST]Update deepseek mtpx acc cases standard Nov 21, 2025
@wangxiyuan wangxiyuan merged commit 41ddb06 into vllm-project:main Nov 24, 2025
23 checks passed
NSDie pushed a commit to NSDie/vllm-ascend that referenced this pull request Nov 24, 2025
### What this PR does / why we need it?
This PR updates the acc standard for deepseek mtpx cases, according to
inner standard
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
By running the test

- vLLM version: v0.11.0
- vLLM main:
vllm-project/vllm@2918c1b

Signed-off-by: jiangyunfan1 <jiangyunfan1@h-partners.com>
Signed-off-by: nsdie <yeyifan@huawei.com>
NSDie pushed a commit to NSDie/vllm-ascend that referenced this pull request Nov 24, 2025
### What this PR does / why we need it?
This PR updates the acc standard for deepseek mtpx cases, according to
inner standard
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
By running the test

- vLLM version: v0.11.0
- vLLM main:
vllm-project/vllm@2918c1b

Signed-off-by: jiangyunfan1 <jiangyunfan1@h-partners.com>
Signed-off-by: nsdie <yeyifan@huawei.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants