Repository navigation
feat(ci): add minimaxai/minimax-m2 to nightly benchmark - #795
Conversation
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (PR)
participant GH as GitHub Actions
participant Tests as Nightly Tests
participant Specs as MODEL_SPECS
participant Runner as Test Runner / Workers
Dev->>GH: Open PR touching e2e_test/benchmarks/**
GH->>GH: Trigger nightly-benchmark workflow (pull_request)
GH->>Tests: Select model matrix (minimaxai/minimax-m2)
Tests->>Specs: Lookup MODEL_SPECS["minimaxai/minimax-m2"]
Specs->>Runner: Provide model path, tp=4, worker_args/vllm_args
Runner->>Tests: Execute http/grpc benchmark jobs (Single/Multi)
Tests->>GH: Upload artifacts / report results
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request integrates the Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![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 the minimaxai/minimax-m2 model to the nightly benchmark suite. The changes correctly configure the model specification and register it for benchmarking. My review includes one suggestion to update the model's feature list in e2e_test/infra/model_specs.py to more accurately reflect its capabilities, ensuring it's included in all relevant tests. This comment aligns with general best practices and does not contradict any specific rules.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-benchmark.yml:
- Line 124: Remove the spaces immediately inside the curly braces for the YAML
list item containing the minimaxai-minimax-m2 entry; update the line "- { id:
minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class:
TestNightlyMinimaxM2Single }" to remove the space after "{" and before "}" (so
the entry becomes "- {id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2,
test_class: TestNightlyMinimaxM2Single}") to satisfy the YAMLlint `braces` rule
while keeping the same keys and values (identifiers: minimaxai/minimax-m2, slug
minimaxai-minimax-m2, test_class TestNightlyMinimaxM2Single).
- Line 124: The multi-worker matrix is missing the generated
TestNightlyMinimaxM2Multi entry, so add an entry with the same repo identifiers
but test_class: TestNightlyMinimaxM2Multi to the multi-worker matrix;
specifically add an item matching id: minimaxai/minimax-m2, slug:
minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi so the multi-worker
nightly path runs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 74e1960f-0ea7-4ee7-956d-df40b7f55e82
📒 Files selected for processing (3)
.github/workflows/nightly-benchmark.ymle2e_test/benchmarks/test_nightly_perf.pye2e_test/infra/model_specs.py
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8941f815f2
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48fd9d607e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| - { id: Qwen/Qwen2.5-7B-Instruct, slug: Qwen-Qwen2.5-7B-Instruct, test_class: TestNightlyQwen7bMulti } | ||
| - { id: Qwen/Qwen3-30B-A3B, slug: Qwen-Qwen3-30B-A3B, test_class: TestNightlyQwen30bMulti } | ||
| - { id: openai/gpt-oss-20b, slug: openai-gpt-oss-20b, test_class: TestNightlyGptOss20bMulti } | ||
| - { id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi } |
There was a problem hiding this comment.
In the multi-worker workflow matrix this new entry schedules TestNightlyMinimaxM2Multi on runs-on: 4-gpu-h100, but the generated test class uses workers(count=2) from e2e_test/benchmarks/test_nightly_perf.py and the new model spec sets tp=4 in e2e_test/infra/model_specs.py; start_workers allocates GPUs sequentially by tp, so the second worker is assigned GPUs 4-7 (see e2e_test/infra/worker.py), which exceeds a 4-GPU host and causes that matrix leg to fail consistently instead of producing benchmark data.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e_test/infra/model_specs.py`:
- Around line 87-88: Update the model mapping key/value to use the canonical
HuggingFace identifier: replace the string "minimaxai/minimax-m2" with
"MiniMaxAI/MiniMax-M2" where the mapping entry currently calls
_resolve_model_path (the dict entry containing "minimaxai/minimax-m2" and the
call to _resolve_model_path should be updated to
_resolve_model_path("MiniMaxAI/MiniMax-M2")). Ensure both the dictionary key (if
used) and the argument passed to _resolve_model_path are corrected to the exact
capitalization shown.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a49a6bf2-407a-47e5-83e5-df0732ff5b56
📒 Files selected for processing (2)
.github/workflows/nightly-benchmark.ymle2e_test/infra/model_specs.py
smfirmin
changed the title
smfirmin
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eeee7ac1b0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
smfirmin
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 23b3a5537f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f9b55b065
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/nightly-benchmark.yml (1)
127-127:⚠️ Potential issue | 🟡 MinorFix flow-map brace spacing to satisfy YAMLlint.
The inline maps on Line 127 and Line 241 have spaces immediately inside
{}and fail thebracesrule.Proposed fix
- - { id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single } + - {id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single} ... - - { id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi } + - {id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi}Also applies to: 241-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly-benchmark.yml at line 127, Fix the YAML linter "braces" rule by removing the spaces inside the inline flow-maps: change occurrences of "{ id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single }" to use no spaces after "{" or before "}" (e.g. "{id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single}"), and apply the same removal of inner-brace spacing for the duplicate inline map instance elsewhere in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-benchmark.yml:
- Around line 16-18: The workflow has pull_request nested under
workflow_dispatch which is invalid; move the pull_request key out so both
workflow_dispatch and pull_request are siblings at the top level of the
job/triggers block. Edit the YAML so workflow_dispatch remains its own mapping
(only containing inputs if present) and add a separate top-level pull_request
mapping (e.g., pull_request: paths: - 'e2e_test/benchmarks/**') alongside
workflow_dispatch to register the PR trigger correctly.
---
Duplicate comments:
In @.github/workflows/nightly-benchmark.yml:
- Line 127: Fix the YAML linter "braces" rule by removing the spaces inside the
inline flow-maps: change occurrences of "{ id: minimaxai/minimax-m2, slug:
minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single }" to use no spaces
after "{" or before "}" (e.g. "{id: minimaxai/minimax-m2, slug:
minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single}"), and apply the
same removal of inner-brace spacing for the duplicate inline map instance
elsewhere in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1706bc0a-5c17-4679-92cc-cbd5c85b5729
📒 Files selected for processing (1)
.github/workflows/nightly-benchmark.yml
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/nightly-benchmark.yml (1)
127-127:⚠️ Potential issue | 🟡 MinorFix flow-map brace spacing to satisfy YAMLlint.
Both added matrix entries still use spaces inside
{}and are flagged by lint.Proposed fix
- - { id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single } + - {id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single} ... - - { id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi } + - {id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Multi}Also applies to: 241-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/nightly-benchmark.yml at line 127, YAML linter is failing due to spaces inside flow-map braces; remove the inner spaces for the matrix entries so they read without spaces inside the braces (e.g. change "{ id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single }" to "{id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single}" and make the same adjustment for the other similar entry), ensuring all flow-map brace spacing complies with yamllint.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/nightly-benchmark.yml:
- Around line 16-18: Update the pull_request.paths for the nightly-benchmark
workflow so changes to benchmark wiring and this workflow itself trigger the
job: modify the pull_request.paths entry (pull_request.paths) to include
e2e_test/infra/model_specs.py and the workflow file (for example add
'e2e_test/infra/model_specs.py' and '.github/workflows/nightly-benchmark.yml' or
broaden to 'e2e_test/**') in addition to the existing 'e2e_test/benchmarks/**'
pattern so PRs that change model wiring or the workflow also run the benchmark
validation.
---
Duplicate comments:
In @.github/workflows/nightly-benchmark.yml:
- Line 127: YAML linter is failing due to spaces inside flow-map braces; remove
the inner spaces for the matrix entries so they read without spaces inside the
braces (e.g. change "{ id: minimaxai/minimax-m2, slug: minimaxai-minimax-m2,
test_class: TestNightlyMinimaxM2Single }" to "{id: minimaxai/minimax-m2, slug:
minimaxai-minimax-m2, test_class: TestNightlyMinimaxM2Single}" and make the same
adjustment for the other similar entry), ensuring all flow-map brace spacing
complies with yamllint.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 799e9d2a-ebb5-4f26-8c02-f240a82bb956
📒 Files selected for processing (1)
.github/workflows/nightly-benchmark.yml
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa49cfdf96
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
https://github.com/lightseekorg/smg/actions/runs/23280593895/job/67693683519?pr=795 |
|
worker log |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e_test/benchmarks/test_nightly_perf.py`:
- Line 104: The MinimaxM2 benchmark entry is configured with multi_workers=4 for
a model with tp=4 which causes CUDA OOM at startup; locate the tuple for
("minimaxai/minimax-m2", "MinimaxM2", 4, ["http", "grpc"], {}) and reduce its
multi-worker count to a feasible value (e.g., set multi_workers from 4 to 2) or
disable multi-worker mode for "MinimaxM2" temporarily until runner
capacity/placement is adjusted so the nightly multi benchmark can start
reliably.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 19fb5c4b-d6bf-49dc-a1ee-c86d31cf597b
📒 Files selected for processing (1)
e2e_test/benchmarks/test_nightly_perf.py
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
5e59948 to
5ff333e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ff333e88e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| _variants = [("Single", 1)] | ||
| if _multi_workers > 0: | ||
| _variants.append(("Multi", _multi_workers)) |
There was a problem hiding this comment.
Fresh evidence: this commit now sets Minimax’s worker count to 0, and the new guard here skips generating any TestNightlyMinimaxM2Multi class at collection time. That means nightly benchmarking can only exercise the single-worker path for this model, so routing/load-balancing regressions that appear only with multiple workers will never be measured even before the workflow matrix is considered.
Useful? React with 👍 / 👎.
Signed-off-by: Sydney Firmin <sydney.firmin@oracle.com>
5ff333e to
7c2ed96
Compare
Signed-off-by: key4ng <rukeyang@gmail.com>
Signed-off-by: key4ng <rukeyang@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b922872bae
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name: "${{ matrix.model.id }} / single-${{ matrix.variant.id }}" | ||
| needs: build-wheel | ||
| if: ${{ !cancelled() }} | ||
| if: false |
There was a problem hiding this comment.
Setting the single-worker job condition to if: false unconditionally skips that entire H100 matrix, and this commit makes the same change for multi-worker as well. As a result, nightly/scheduled/manual runs stop executing the H100 benchmark suites (Llama/Qwen/GPT-OSS paths), so regressions on those model/runtime combinations are no longer detected and their benchmark artifacts disappear.
Useful? React with 👍 / 👎.
Signed-off-by: key4ng <rukeyang@gmail.com>
2 participants
• ## Description
Problem
minimaxai/minimax-m2needs to be benchmarked.Solution
Add
minimaxai/minimax-m2to the nightly benchmark path end to end: define its E2E model spec, register nightly single/multi benchmark test classes, and include it in the nightly benchmark workflow matrix.Changes
minimaxai/minimax-m2toe2e_test/infra/model_specs.py.tp=4and--trust-remote-codefor both worker and vLLM startup.("minimaxai/minimax-m2", "MinimaxM2", 2, ["http", "grpc"], {})to the nightly benchmark model list ine2e_test/benchmarks/test_nightly_perf.py.minimaxai/minimax-m2to the single-worker matrix in.github/workflows/nightly-benchmark.yml.Summary by CodeRabbit