Skip to content

fix(golden-al): validate curves by draft model / fix(golden-al):按草稿模型校验黄金 AL 曲线 - #2394

Closed
xinli-sw wants to merge 1 commit into
mainfrom
minimax_spec_gqa
Closed

fix(golden-al): validate curves by draft model / fix(golden-al):按草稿模型校验黄金 AL 曲线#2394
xinli-sw wants to merge 1 commit into
mainfrom
minimax_spec_gqa

Conversation

@xinli-sw

@xinli-sw xinli-sw commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Keep separate MiniMax-M3 golden AL curves for Inferact/MiniMax-M3-EAGLE3 and Inferact/MiniMax-M3-EAGLE3-GQA.
  • Add canonical draft-model metadata and a deterministic validator keyed by target model, draft model, thinking mode, and draft length.
  • Update the CODEOWNER checklist and verifier so ambiguous, unknown, duplicate, or mismatched AL selections fail.
  • Clarify that fixed-sequence benchmark and eval jobs use real MTP/EAGLE acceptance; golden AL values are pinned for synthetic AgentX throughput.

Validation

  • uv run --with pytest --with pyyaml python -m pytest utils/test_validate_golden_al.py -v — 6 passed.
  • Ruff and committed-ref diff checks passed.
  • Both 16-cell MiniMax curves match their source Actions artifacts.

中文说明

  • Inferact/MiniMax-M3-EAGLE3Inferact/MiniMax-M3-EAGLE3-GQA 分别保留独立的 MiniMax-M3 黄金 AL 曲线。
  • 新增规范化草稿模型元数据和确定性校验器,按目标模型、草稿模型、thinking 模式及 draft 长度选择曲线。
  • 更新 CODEOWNER 审阅清单和验证器;存在歧义、未知或重复草稿模型,以及 AL 数值不匹配时均会失败。
  • 明确固定序列长度基准测试和评估任务使用真实 MTP/EAGLE 接受行为;黄金 AL 仅用于 AgentX 吞吐量任务中固定的合成接受目标。

验证

  • uv run --with pytest --with pyyaml python -m pytest utils/test_validate_golden_al.py -v — 6 项测试通过。
  • Ruff 与已提交引用的差异检查通过。
  • 两条 MiniMax 曲线的各 16 个 AL 数值均与对应的 Actions 源产物一致。

@xinli-sw
xinli-sw requested a review from a team July 28, 2026 19:30
@@ -1,4 +1,4 @@
# Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/28061204145
# # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 This PR bumps the MiniMax-M3 EAGLE3 golden AL source run to 29784780049, but golden_al_distribution/README.md line 114 still links that row to the old run 28061204145, leaving the README and YAML in disagreement about which CI run produced the committed values. Please update the README table's link/run-id to match (the other three rows are kept in sync with their YAML source-run lines, so this is now the sole stale entry).

Extended reasoning...

What the bug is

golden_al_distribution/minimaxm3_eagle3.yaml line 1 records the source GitHub Actions run that produced the committed AL numbers. This PR bumps that run from 28061204145 to 29784780049. However, golden_al_distribution/README.md maintains a "Current golden curves" table (line 114) that separately links each model row to its source run for traceability/audit purposes. That table entry was not updated in this PR and still reads:

| MiniMax-M3 | EAGLE3 | [`minimaxm3_eagle3.yaml`](minimaxm3_eagle3.yaml) | [28061204145](https://github.com/SemiAnalysisAI/InferenceX/actions/runs/28061204145) |

Why existing code/process does not prevent it

The README table is manually maintained, not auto-generated from the YAML source-run comment — there is no CI check or generation script that keeps the two in sync. Nothing in the current review/merge process flags a mismatch between a YAML's first-line source run and the README table's "Source run" column.

Proof this is a real, isolated regression

Cross-checking the other three rows in the same table confirms the table is normally kept byte-for-byte in sync with each YAML's first line:

  • kimik2.5_eagle3.yaml first line run = 28122195822, README = 28122195822
  • dsv4_mtp.yaml first line run = 27180633016, README = 27180633016
  • qwen3.5_mtp.yaml first line run = 27317114007, README = 27317114007
  • minimaxm3_eagle3.yaml first line run (after this PR) = 29784780049, README = 28061204145

So this row is now the sole diverging entry, directly caused by this PR changing the YAML without a matching README update.

Impact

This is documentation-only drift — the benchmarking/comparison code reads the YAML directly, not the README table, so no runtime behavior, build, or test is affected. The practical impact is that a reviewer or future engineer following the README's link to "verify the source run" for MiniMax-M3 EAGLE3 will land on the superseded workflow run (28061204145) instead of the run that actually produced the currently-committed values (29784780049), undermining the auditability the README table exists to provide. Note README_zh.md likely carries the same stale table and should be checked too.

Suggested fix

Update README.md (and README_zh.md if present) line 114 to link to 29784780049:

| MiniMax-M3 | EAGLE3 | [`minimaxm3_eagle3.yaml`](minimaxm3_eagle3.yaml) | [29784780049](https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049) |

This is a one-line fix and does not need to block merging, since no functional path is affected — but it is worth including in this PR (or a fast follow-up) to keep the traceability table accurate, especially since a prior identical golden-AL update (commit 873a954) also missed syncing this README table, suggesting this is a recurring oversight in this workflow.

@@ -1,4 +1,4 @@
# Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/28061204145
# # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Line 1 of golden_al_distribution/minimaxm3_eagle3.yaml now reads # # Source GitHub Actions run: ... with a doubled # prefix, while every other file in golden_al_distribution/ uses a single #. It's a cosmetic copy-paste artifact that doesn't affect YAML parsing or the AL values, but it should be fixed to keep the auditable source-run pointer consistent with the established convention.

Extended reasoning...

This PR updates the golden AL reference values for minimax-m3 after a re-run of the SPEED-Bench workflow against a new EAGLE3 head (Inferact/MiniMax-M3-EAGLE3-GQA), and as part of that update it rewrites the audit-pointer comment on line 1 to point at the new GitHub Actions run URL (.../runs/29784780049). In doing so, the line was changed from:

# Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049

to:

# # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049

with an extra # prepended before Source. This is almost certainly a mechanical artifact — the automation/script that regenerates this header line appears to have re-prepended a # comment marker onto a line that was already commented, rather than replacing the whole line.

Why this wasn't caught elsewhere: the Source GitHub Actions run line is prepended by a step separate from the main header-generation logic in benchmarks/single_node/speedbench/minimaxm3_fp4_b300_vllm.sh (which emits the Acceptance Length (AL) line onward). Nothing validates that this specific line retains exactly one #, so the doubling passed through unnoticed. I confirmed by checking head -1 across all golden_al_distribution/*.yaml files (dsv4_mtp.yaml, kimik2.5_eagle3.yaml, kimik3_dspark.yaml, kimik3_dspark_probabilistic*.yaml, qwen3.5_mtp.yaml) that every other file uses a single # prefix — minimaxm3_eagle3.yaml is the sole outlier introduced by this PR.

Impact: none functionally. This is a YAML comment; it has zero effect on parsing, and no tooling in the repo consumes this line programmatically (it's purely a human-readable audit pointer, documented in the golden_al_distribution README as the way to trace which CI run produced the pasted values). So the golden AL values themselves are correct and usable as-is.

Proof/repro:

  1. head -1 golden_al_distribution/minimaxm3_eagle3.yaml on main (pre-PR) → # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/28061204145
  2. Same command post-PR → # # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049
  3. head -1 golden_al_distribution/*.yaml for any of the other 5 files → single # prefix in each case, confirming this file alone deviates.

Fix: simply drop the extra # so the line reads # Source GitHub Actions run: https://github.com/SemiAnalysisAI/InferenceX/actions/runs/29784780049, matching the convention used by every other file in the directory. Since it's purely cosmetic and doesn't affect any consumed value, this doesn't need to block the merge, but it's a quick one-line fix worth making before or shortly after merge to keep the audit trail clean.

Add canonical draft-model metadata and deterministic AL validation. Update the bilingual guidance and CODEOWNER verifier policy.

中文:添加规范化草稿模型元数据和确定性的 AL 校验,并同步更新中英文指南与 CODEOWNER 验证策略。
@Ankur-singh Ankur-singh changed the title [goldenAL] update golden AL for MiniMax-M3 Eagle head to Inferact/MiniMax-M3-EAGLE3-GQA- #2393 fix(golden-al): validate curves by draft model / fix(golden-al):按草稿模型校验黄金 AL 曲线 Jul 29, 2026
@functionstackx

Copy link
Copy Markdown
Collaborator

replaced by #2413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants