CI: Initial Aiter benchmark tests - #2115
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new GitHub Actions workflow to run Aiter benchmark tests using a reusable workflow from the ROCm/ATOM repository. The workflow is configured to test the DeepSeek-R1-0528 model with specific parameters including FP8 KV cache and 8-way tensor parallelism.
Changes:
- Added new CI workflow
.github/workflows/aiter-benchmark.yamlthat calls an external reusable workflow from ROCm/ATOM - Configured to run on pull requests and manual dispatch (marked as "for testing")
- Sets up benchmark parameters for DeepSeek-R1-0528 model testing
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: Aiter Benchmark | ||
|
|
||
| on: | ||
| pull_request: # for testing |
There was a problem hiding this comment.
The comment "for testing" suggests this is a temporary configuration. Most other CI workflows in the repository include a push trigger for the main branch in addition to pull_request. Consider whether the push trigger should be added for consistency with workflows like aiter-test.yaml, atom-test.yaml, vllm_benchmark.yaml, and sglang_downstream.yaml, which all trigger on both push and pull_request events.
| pull_request: # for testing | |
| push: | |
| branches: [main] | |
| paths-ignore: | |
| - '**/*.md' | |
| - 'docs/**' | |
| - 'LICENSE' | |
| - '.gitignore' | |
| pull_request: |
| name: Aiter Benchmark | ||
| uses: ROCm/ATOM/.github/workflows/atom-benchmark.yaml@atom_benchmark_expose | ||
| with: | ||
| deepseek-r1-0528: true |
There was a problem hiding this comment.
The parameter name 'deepseek-r1-0528' with a boolean value 'true' suggests this is a feature flag. However, this doesn't match the typical model naming conventions seen in other workflows. In atom-test.yaml, the model is referenced as "DeepSeek-R1-0528" (line 49-51) with corresponding model_path "deepseek-ai/DeepSeek-R1-0528". Verify that the reusable workflow 'ROCm/ATOM/.github/workflows/atom-benchmark.yaml@atom_benchmark_expose' expects this exact parameter name and format. If this is meant to specify a model, consider using a more descriptive parameter name like 'model_name' or 'enable_deepseek_r1_0528' to make the intent clearer.
| deepseek-r1-0528: true | |
| enable_deepseek_r1_0528: true |
| jobs: | ||
| aiter_benchmark: | ||
| name: Aiter Benchmark | ||
| uses: ROCm/ATOM/.github/workflows/atom-benchmark.yaml@atom_benchmark_expose |
There was a problem hiding this comment.
The workflow references a reusable workflow at branch 'atom_benchmark_expose'. Verify that this branch exists in the ROCm/ATOM repository and is the intended stable reference. Using a branch name (rather than a tag or commit SHA) means the workflow behavior could change if the branch is updated. Consider whether this should reference a specific tag or commit for stability, especially if this is intended for production use beyond testing.
| uses: ROCm/ATOM/.github/workflows/atom-benchmark.yaml@atom_benchmark_expose | |
| uses: ROCm/ATOM/.github/workflows/atom-benchmark.yaml@0123456789abcdef0123456789abcdef01234567 |
| with: | ||
| deepseek-r1-0528: true | ||
| extra_args: "--kv_cache_dtype fp8 -tp 8" | ||
| param_list: "1024,1024,128,0.8" No newline at end of file |
There was a problem hiding this comment.
The parameter 'param_list' with value "1024,1024,128,0.8" is passed without any documentation or comments explaining what these comma-separated values represent. Based on typical benchmark parameters, these could be input_length, output_length, batch_size, and some ratio, but this is not clear. Consider adding an inline comment to document what these parameters mean, similar to how other workflows document their configurations.
| param_list: "1024,1024,128,0.8" | |
| param_list: "1024,1024,128,0.8" # input_length, output_length, batch_size, request_ratio |
| with: | ||
| deepseek-r1-0528: true | ||
| extra_args: "--kv_cache_dtype fp8 -tp 8" | ||
| param_list: "1024,1024,128,0.8" No newline at end of file |
There was a problem hiding this comment.
When calling a reusable workflow from an external repository (ROCm/ATOM), secrets are not automatically inherited and must be explicitly passed. Many workflows in this repository use secrets like DOCKER_PASSWORD and HF_TOKEN_TEST. If the reusable atom-benchmark.yaml workflow requires any secrets (e.g., for Docker login or model downloads), you need to add a 'secrets:' section to pass them explicitly, such as 'secrets: inherit' or by passing specific secrets.
| param_list: "1024,1024,128,0.8" | |
| param_list: "1024,1024,128,0.8" | |
| secrets: inherit |
No description provided.