[CI] Add multi-node test case for a2 #3805
Conversation
Signed-off-by: wangli <[email protected]>
Signed-off-by: wangli <[email protected]>
|
👋 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. |
![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 adds a multi-node test case for the DeepSeek-R1-W8A8 model. The changes correctly parameterize the NPU resource requests in the Kubernetes job template. However, the new test configuration file is missing a required benchmarks section, which will cause a runtime failure. I have added a critical review comment to address this issue.
| --enforce-eager | ||
| --speculative-config '{"num_speculative_tokens": 1, "method":"deepseek_mtp"}' \ | ||
| --additional-config '{"ascend_scheduler_config":{"enabled":false},"chunked_prefill_for_mla":true,"enable_weight_nz_layout":true}' | ||
| benchmarks: |
There was a problem hiding this comment.
The benchmarks section is empty. The test configuration parser in tests/e2e/nightly/multi_node/config/multi_node_config.py requires perf and acc commands to be defined under this key. An empty benchmarks: key is parsed as None, which will cause an AssertionError at runtime when the configuration is loaded. Please add the required benchmark commands to proceed.
benchmarks:
perf: "echo 'perf command placeholder'"
acc: "echo 'acc command placeholder'"
|
This should merged after #3764 |
2 participants
What this PR does / why we need it?
This patch add multi-node test case for a2
Does this PR introduce any user-facing change?
How was this patch tested?