MI355X MiniMax M3 MTP - #2458
Conversation
|
Thanks for the contribution! Please reach out to respective companies' CODEOWNER to fill in the latest PR_REVIEW_CHECKLIST.md before pinging core maintainer on Slack for review. In order for the signoff PR check bot to trigger, you must follow the PR_REVIEW_CHECKLIST.md template correctly, including the phrase For PR verification, add the PR authors are responsible for ensuring that after merging, all GitHub Action jobs fully pass. A lot of the time, failures are just flakes and simply re-running the failed jobs will fix it. See GitHub's docs on re-running failed jobs 感谢你的贡献！请联系相应公司的 CODEOWNER 填写最新的 PR_REVIEW_CHECKLIST.md，然后再在 Slack 上联系核心维护者进行审阅。为了触发 signoff PR 检查机器人，你必须正确遵循 PR_REVIEW_CHECKLIST.md 模板，包括保留英文语句 如需进行 PR 验证，请为此 PR 添加 PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动（flake），重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档 |
e49338f to
94074fb
Compare
cquil11
changed the title
94074fb to
100b98b
Compare
|
Claude finished @cquil11's task in 1m 24s —— View job Review
LGTM - no blocking issues found Verified: the new entry is appended at the end of |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM aside from the inline nit — this is a minor, low-risk config-only change.
Extended reasoning...
Overview
The PR adds a single new changelog entry to perf-changelog.yaml (lines 5359-5363), registering the minimaxm3-fp4-mi355x-vllm-agentic config for its initial MI355X MiniMax M3 submission via the new AgentX harness. No other files are touched. I confirmed this is genuinely the first changelog entry for this config key (only other reference is the config definition itself in configs/amd-master.yaml), so the 'Initial' description is accurate and the ruled-out mislabeling concern does not apply.
Security risks
None. This is a declarative YAML changelog entry with no code execution, credentials, or user input involved.
Level of scrutiny
Low. This follows the repo's well-established append-only changelog pattern (list of config-keys/description/pr-link entries) with no schema changes or logic modifications. The change is purely additive and mechanical.
Other factors
The one inline finding is a nit about the PR title/description missing the bilingual (English/Chinese) format mandated by AGENTS.md — a real, actionable process gap, but purely metadata and not something that affects correctness or functionality. Recent merged PRs (#2457, #2455) show this convention is not currently CI-enforced, so it's not a blocker here either. No outstanding unaddressed reviewer comments exist on this PR.
| - config-keys: | ||
| - minimaxm3-fp4-mi355x-vllm-agentic | ||
| description: | ||
| - "Initial MI355X MiniMax M3 with new AgentX harness" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/2458 |
There was a problem hiding this comment.
🟡 This PR's title and description are English-only, but AGENTS.md mandates bilingual PR titles (<English title> / <中文标题>) and a ## 中文说明 section in the body for every PR / 本 PR 的标题和描述仅为英文，但 AGENTS.md 要求所有 PR 标题必须为双语格式（<英文标题> / <中文标题>），并在正文中包含 ## 中文说明 部分。请补充中文标题和说明以符合仓库规范。
Extended reasoning...
AGENTS.md line 7 (a checked-in, mandatory repository convention, not subjective style) states verbatim: "PR and GitHub-issue titles & descriptions must be bilingual — include a Simplified Chinese version in addition to English. Title format: <English title> / <中文标题>. In the PR/issue body, follow the English content with its Chinese translation (e.g. a ## 中文说明 section mirroring the summary...). ... This applies to every PR and every issue."
This PR's title is perf(agentx): refresh MI355X MiniMax M3 — there is no / <中文标题> suffix. Its body has only ## Summary and ## Validation sections, both in English, with no ## 中文说明 section translating the summary. The only documented exceptions in AGENTS.md are the CODEOWNER sign-off template (which must stay English-verbatim so the sign-off verifier bot can match it) and bot-generated comments following their own workflow templates — neither applies here, since this is a human-authored PR title and description.
Step-by-step proof:
- AGENTS.md:7 requires: title format
<English> / <中文>, and body with an English section followed by a## 中文说明mirror. - PR MI355X MiniMax M3 MTP #2458 title:
perf(agentx): refresh MI355X MiniMax M3→ no/separator, no Chinese text at all. - PR MI355X MiniMax M3 MTP #2458 body:
## Summary(3 bullets, English) +## Validation(2 bullets, English) → no## 中文说明section exists anywhere in the body. - Checking the stated exceptions: CODEOWNER sign-off template (not applicable — no sign-off here) and bot comment templates (not applicable — this is the PR author's own title/description). Neither exempts this PR.
- Conclusion: the PR violates the checked-in bilingual convention as written, with no applicable exception.
Why existing tooling doesn't catch this: there's no CI gate that lints PR titles/descriptions for a Chinese section — this convention is enforced only by human/agent review, not automation. That's corroborated by recent history: PR #2457 (fix(agentx): pin AIPerf v1 timing watchdog) and PR #2455 also merged with English-only titles, showing the rule is currently unenforced by any hard gate.
Impact of merging as-is: purely PR-metadata hygiene — the underlying YAML change (adding the MiniMax M3 config entry) is unaffected and nothing breaks functionally. Fix is trivial: the author (or a maintainer/bot) can edit the PR title to perf(agentx): refresh MI355X MiniMax M3 / perf(agentx)：更新 MI355X MiniMax M3 and append a ## 中文说明 section mirroring the Summary/Validation bullets.
Severity: since this causes no runtime failure, crash, or data loss, and AGENTS.md does not declare the bilingual rule a hard merge-blocker (nor is it CI-enforced), this is a nit — a real, actionable convention violation worth flagging, but not one that should hold up the merge.
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30777851549 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30777851549 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30834201753 |
functionstackx
left a comment
functionstackx
left a comment
There was a problem hiding this comment.
@cquil11 can this be with EAGLE?
cquil11
changed the title
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30843862008 |
|
Updated the MI355X recipe to EAGLE3-GQA with 3 speculative tokens and the committed thinking-on golden AL 2.78 for throughput; eval-only retains real verification. Replacement sweep: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/30846281719 / 已将 MI355X 配方更新为 EAGLE3-GQA，使用 3 个投机 token；吞吐量测试采用已提交的 thinking-on 黄金接受长度 2.78，eval-only 保留真实校验。替代扫描：https://github.com/SemiAnalysisAI/InferenceX/actions/runs/30846281719 |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30846281719 |
Align the MI355X MXFP4 server with the supported MiniMax-M3 ROCm and EAGLE3-GQA configuration: block-128 Triton attention for target and draft, FP8 KV cache, AITER MoE, INT4 QuickReduce, three speculative tokens, and the validated synthetic acceptance length. Keep TP4 concurrency 1-24 GPU-resident, add a validated native DRAM offload point at concurrency 28, and add TEP8 concurrency 20, 28, and 36 for the higher-throughput range. Rename the benchmark and configuration as the MTP variant. 中文：将 MI355X MXFP4 服务配置与受支持的 MiniMax-M3 ROCm 和 EAGLE3-GQA 方案对齐：目标模型与草稿模型均使用 block-128 Triton 注意力，并启用 FP8 KV 缓存、AITER MoE、INT4 QuickReduce、3 个投机 token 以及经验证的合成接受长度。 TP4 并发 1-24 保持 KV 缓存驻留 GPU，在并发 28 增加已验证的原生 DRAM KV 卸载点，并增加 TEP8 并发 20、28、36 以覆盖高吞吐区间。基准脚本和配置重命名为 MTP 版本。
Append the performance changelog entry for the corrected AgentX harness. 中文：追加使用修正后 AgentX 测试框架的性能变更日志条目。
972bb3b to
73e2f4f
Compare
cquil11
changed the title
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=30937012069 |
functionstackx
changed the title
|
Closing in favor of #2487 |
2 participants
Summary
Validation
中文说明
验证