-
Notifications
You must be signed in to change notification settings - Fork 617
[TEST]Update deepseek mtpx acc cases standard #4321
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
Conversation
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
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.
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.
| 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} |
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.
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
-
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] |
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.
Signed-off-by: jiangyunfan1 <jiangyunfan1@h-partners.com>
### 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>
### 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>
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