Skip to content

[VideoGenX] run H3 through native sweeps with measured power - #2916

Open
edwingao28 wants to merge 9 commits into
mainfrom
feat/h3-cross-hardware
Open

edwingao28 wants to merge 9 commits into
mainfrom
feat/h3-cross-hardware

Conversation

@edwingao28

@edwingao28 edwingao28 commented Sep 9, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Run H3 through native srt-slurm sweeps with sealed artifacts and measured GPU power. Supersedes #2894.

Validation Result / evidence
Formal sweep, source 752a76b7, checkout e37ed63d H200/B200 passed; 24 measured clips, 6 warmups, 126 sealed files. All media decoded; energy recomputation and complete manifest passed. Canary skipped.
H200, C1/C2/C4 Client-ready P50: 117.106/233.548/292.273 s; approximately 1.38 kW across two participating GPUs.
B200, C1/C2/C4 Client-ready P50: 59.220/118.525/148.140 s; approximately 1.96 kW across two participating GPUs.
CPU CI, 76dfbbf4 Tests, lint, workflow security and H3 harness/power passed. Tests/docs-only follow-up; no replacement GPU execution.

Limits: Four measurements/cell are History-only. Trusted reuse requires #4274; public runtime images, quality and production publication remain unverified.

PR checklist and change type

Type of Change

  • Bug fix
  • New feature
  • Configuration change
  • Documentation update
  • Other (please describe)

Checklist

  • I have tested my changes locally
  • I have updated documentation if necessary
  • If python -m infx.matrix.changed --base origin/main lists affected config keys, I have appended one new entry covering them to the physical end of inferencex-e2e/perf-changelog.yaml; if it lists none, I have added none. I have not edited historical entries
  • If this PR appends an inferencex-e2e/perf-changelog.yaml entry, it carries exactly one primary sweep label (a maintainer applies it on fork PRs): full-sweep-fail-fast (recommended), full-sweep-enabled, or non-canary-full-sweep-enabled. Optional modifiers all-evals, evals-only, and agentx-fast require a primary label; the last two block reuse while applied.
  • Before merging via reuse, an authorized maintainer (OWNER/MEMBER/COLLABORATOR) has commented /use <run_id> (or the legacy /reuse-sweep-run) on this PR. Do this only once there is a final full sweep that is all green with evals passing, since after this comment the primary sweep label will no longer automatically kick off new sweeps. Remove and re-add the primary sweep label to force a new sweep.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

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 As a PR reviewer and CODEOWNER, I have reviewed this and have.

For PR verification, add the full-sweep-fail-fast label (strongly recommended) to this PR — the benchmark sweep only runs on labeled PRs. Use full-sweep-enabled only if you need matrix jobs to keep running past a failure.

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 模板,包括保留英文语句 As a PR reviewer and CODEOWNER, I have reviewed this and have。

如需进行 PR 验证,请为此 PR 添加 full-sweep-fail-fast 标签(强烈推荐)— 基准测试 sweep 仅在带有标签的 PR 上运行。仅当需要矩阵任务在失败后继续运行时才使用 full-sweep-enabled。

PR 作者有责任确保合并后所有 GitHub Action 任务完全通过。 很多时候失败只是偶发抖动(flake),重新运行失败的任务即可解决。参见 GitHub 关于重新运行失败任务的文档

@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch from a7facf5 to 7019bb2 Compare September 9, 2026 08:42
@edwingao28 edwingao28 changed the title [H3 CI] add hardware sites and serving matrices / 接入硬件站点与服务测量 [H3 CI] compare frozen hardware runs and retain serving evidence / 保留跨硬件服务证据 Sep 9, 2026
@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch from 2327589 to 8247212 Compare September 27, 2026 06:32
@edwingao28 edwingao28 changed the title [H3 CI] compare frozen hardware runs and retain serving evidence / 保留跨硬件服务证据 [H3] integrate manual video benchmarks with the current infx planner / [H3] 将手动视频基准测试接入当前 infx planner Sep 27, 2026
@edwingao28
edwingao28 changed the base branch from feat/h3-video-ci to main September 27, 2026 06:33
@functionstackx

Copy link
Copy Markdown
Collaborator

Sorry, over the weekend, there was 2 major refactors to clean up the technical debt accumalated over the past 11 months of moving at the speed of light. We don't see any major refactors in the forthseeable future besides cleaning up AMD multinode AgentX pile of bash. As much, due to the refactors, u would need to ask your agent to rebase from remote main@latest. Thank you in advance for ur understanding

cursor Bot pushed a commit that referenced this pull request Sep 28, 2026
补充 #2916 与 App #1193 的契约矩阵、CPU 验证结果与下一步决策说明。

Co-authored-by: Wenyao Gao <edwingao28@users.noreply.github.com>
@edwingao28 edwingao28 changed the title [H3] integrate manual video benchmarks with the current infx planner / [H3] 将手动视频基准测试接入当前 infx planner [Video] integrate manual video benchmarks with the current infx planner / 将手动视频基准测试接入当前 infx planner Oct 5, 2026
edwingao28 added a commit that referenced this pull request Oct 17, 2026
…aluator

Carries the MiniMax-H3 serving harness from #2916 onto main. The evaluator
gains srt-serve/srt-client/srt-finalize so a generic srt-slurm service runs
the diffusion server, each concurrency publishes its own measurement window,
and the dashboard bundle is assembled from the native power package.

中文:将 #2916 的 MiniMax-H3 serving harness 迁移到 main。evaluator 新增
srt-serve/srt-client/srt-finalize:由 srt-slurm generic service 启动 diffusion
server,每个并发度发布自己的测量窗口,并基于原生功耗包组装仪表板产物。
@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch 5 times, most recently from 4aa0638 to ddc4127 Compare October 17, 2026 04:18
edwingao28 added a commit that referenced this pull request Oct 17, 2026
…aluator

Carries the MiniMax-H3 serving harness from #2916 onto main. The evaluator
gains srt-serve/srt-client/srt-finalize so a generic srt-slurm service runs
the diffusion server, each concurrency publishes its own measurement window,
and the dashboard bundle is assembled from the native power package.

中文:将 #2916 的 MiniMax-H3 serving harness 迁移到 main。evaluator 新增
srt-serve/srt-client/srt-finalize:由 srt-slurm generic service 启动 diffusion
server,每个并发度发布自己的测量窗口,并基于原生功耗包组装仪表板产物。
@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch from ddc4127 to f7c7f58 Compare October 17, 2026 04:31
…aluator

Carries the MiniMax-H3 serving harness from #2916 onto main. The evaluator
gains srt-serve/srt-client/srt-finalize so a generic srt-slurm service runs
the diffusion server, each concurrency publishes its own measurement window,
and the dashboard bundle is assembled from the native power package.

中文:将 #2916 的 MiniMax-H3 serving harness 迁移到 main。evaluator 新增
srt-serve/srt-client/srt-finalize:由 srt-slurm generic service 启动 diffusion
server,每个并发度发布自己的测量窗口,并基于原生功耗包组装仪表板产物。
Adds the video-generation scenario to the vendor master configs and routes it
to a new srt-video launch path: the recipe binds a custom benchmark plus the
h3-server service, the job mounts the H3 package and HF cache, and
infx.results.video validates every concurrency against the native power
package with per-clip energy before the bundle is built.

中文:在厂商 master 配置中加入 video-generation scenario,并路由到新的
srt-video 启动路径:recipe 绑定 custom benchmark 与 h3-server service,作业挂载
H3 package 与 HF cache,infx.results.video 按并发度用原生功耗包校验并以每个
clip 归一化能耗,然后再组装产物。
@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch from f7c7f58 to 068d64f Compare October 17, 2026 04:42
e2e-tests.yml plans video-generation rows into a test-sweep-video job, the
benchmark template passes the H3 spec and concurrency list to the launcher and
uploads the h3-results bundle, and the standalone H3 workflows use the
harness's check_env.sh. One perf-changelog entry covers the four H3 keys.

中文:e2e-tests.yml 将 video-generation 行规划到 test-sweep-video 作业,benchmark
模板向 launcher 传递 H3 spec 与并发度列表并上传 h3-results 产物,独立的 H3
workflow 改用 harness 的 check_env.sh。perf-changelog 新增一条覆盖四个 H3 键。
@edwingao28
edwingao28 force-pushed the feat/h3-cross-hardware branch from 068d64f to 0cb4c84 Compare October 17, 2026 04:45
@edwingao28 edwingao28 changed the title [Video] integrate manual video benchmarks with the current infx planner / 将手动视频基准测试接入当前 infx planner [VideoGenX] run H3 video generation through the native srt-slurm launcher / 通过原生 srt-slurm launcher 运行 H3 视频生成 Oct 17, 2026
@edwingao28
edwingao28 marked this pull request as ready for review October 17, 2026 06:44
@edwingao28
edwingao28 requested a review from a team October 17, 2026 06:44
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

inferencex-e2e/infx diff: +1590 -23

Generally most InferenceX submissions don't need to add changes to the infx package. On the off chance they do, generally the diff is small. Please prompt your agent to clean your PR to keep your diff to only necessary & minimal changes too. We have an increased code quality bar for changes to the infx package.

中文

大多数 InferenceX 提交通常不需要修改 infx 包。即使确实需要,改动通常也很小。请让你的 agent 清理 PR,使改动只包含必要且最小的内容。infx 包的改动有更高的代码质量要求。

@claude

claude Bot commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @edwingao28's task in 1m 54s —— View job


PR review

  • Read AGENTS.md and gather context
  • Review launcher, matrix, results and power changes
  • Check master configs (images, prefixes), perf-changelog, generate.py line count
  • Post findings

I found 2 blocking issues and 1 warning, all in the config and changelog layer. I found no bugs in the launcher or results code.

  1. 🔴 The H3 images aren't public (nvidia-master.yaml L9757/L9776/L9793, amd-master.yaml L1740). The config comments say they are "not pullable from a registry". Both images are squashes pre-staged on the clusters, so the results can't be reproduced and stage_image fails on any cluster without the squash cached. Fix: push the runtimes to Docker Hub or GHCR.
  2. 🔴 The MI355X row can't run: it uses the CUDA H200 image and the CUDA spec on cluster:mi355x-amds, and the harness README itself says AMD generation is unsupported. Fix: remove the key until a ROCm runtime exists.
  3. 🟡 The changelog lists the H100 and MI355X keys, which the PR says are blocked, so the sweep that gates /use can't go green. Fix: delete lines 10079–10080 of perf-changelog.yaml so the entry lists only h200 and b200. ⚠️ Don't apply the suggestion block in that inline comment: it re-adds the h200/b200 lines and would duplicate them.

Other checks

  • Launcher and results code: no problems. I checked run_video, finish_video and finalize_video_results against the single-node path, the srt-video routing, VideoRequest, the binder in infx/srt_slurm/video.py, the per-concurrency join in infx/results/video.py, clip energy normalization in power/common.py and multinode.py (the per-token division by zero is guarded), the matrix and schema plumbing, and the benchmark-tmpl.yml env wiring.
  • Images: they reach the job through stage_image, so no driver hand-rolls enroot import.
  • Model prefix: the h3 prefix has a MODELS.md entry.
  • Changelog position: the entry is appended at the end of the file.
  • Line count: generate.py is +62 lines (1766 → 1828).
  • Not run: I didn't run the test suite; the PR reports CI green at 0cb4c847d.

Comment on lines +9749 to +9757
# (experimental/video-generation/specs) served by a fixed deployment, every listed
# concurrency against the same server. Rows run through the native srt-slurm launcher
# (infx.launch path srt-video) with the cluster's GPU power exporter; the H3 client and
# server come from experimental/video-generation, mounted into the job as /h3-package.
# The image is the prepared H3 runtime squash staged in the cluster's squash cache
# (sglang 71de97b with the H3 server-timing patch and the evaluator's media
# dependencies); it is not pullable from a registry.
h3-bf16-h200-sglang-video:
image: hongccc/sglang-omni:h3-runtime-71de97b

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.

🔴 BLOCKING: The image isn't publicly accessible. The comment above says so ("it is not pullable from a registry"), and the PR body calls both H3 images "pre-staged squashes on the clusters, not registry images". The same problem applies to semianalysis/sglang-h3:b200-cu130-71de97b (L9776), the H100 row (L9793) and amd-master.yaml L1740.

Why it matters: run_video stages the image with run.backend.stage_image(request.image, ...). That works only where a matching squash already sits in the cache. On any other cluster, a cache eviction, or an outside reproduction, the docker:// import fails or resolves to something other than the validated runtime. Since nobody can pull the image, nobody can reproduce these benchmark numbers.

Fix: Push the H3 runtimes (SGLang 71de97b plus the evaluator's media deps, CUDA 12 and CUDA 13 variants) to a public registry such as Docker Hub semianalysis/... or GHCR. Then point image: at those tags and drop the "not pullable" comment. Image must be publicly accessible on NGC, Docker Hub, or another public registry. Local paths like /scratch/... or .sqsh files are generally not accepted.

Comment thread inferencex-e2e/configs/amd-master.yaml Outdated
Comment on lines +1739 to +1754
h3-bf16-mi355x-sglang-video:
image: hongccc/sglang-omni:h3-runtime-71de97b
model: MiniMaxAI/MiniMax-H3
model-prefix: h3
runner: cluster:mi355x-amds
precision: bf16
framework: sglang
multinode: false
srt-recipe-dir: h3/sglang/mi355x-bf16
scenarios:
video-generation:
- spec: h3-serving-2gpu.json
search-space:
- gpus: 2
srt-recipe: serving-client.yaml
conc-list: [1, 2, 4]

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.

🔴 BLOCKING: This MI355X row can't run. It points at the CUDA runtime hongccc/sglang-omni:h3-runtime-71de97b, the same image as H200, and at the CUDA spec h3-serving-2gpu.json (/usr/bin/python3 in that CUDA image). experimental/video-generation/README.md L259 states that "actual AMD generation remains unsupported", and the PR body says MI355X is "blocked on the AMD runtime".

Why it matters: The key is listed in the new perf-changelog.yaml entry, so the PR sweep, and the post-merge run that AGENTS.md makes the author responsible for, will schedule h3-bf16-mi355x-sglang-video on cluster:mi355x-amds. That job is guaranteed to fail. If it ever gets through, it is not measuring a ROCm build at all.

Fix: Remove h3-bf16-mi355x-sglang-video (and its mi355x-bf16/serving-client.yaml recipe, if you like) from this PR and from the changelog's config-keys. Add it back in a follow-up once there is a ROCm H3 runtime image and a matching spec.

Comment thread inferencex-e2e/perf-changelog.yaml Outdated
Comment on lines +10079 to +10080
- h3-bf16-h100-sglang-video
- h3-bf16-mi355x-sglang-video

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.

🟡 WARNING: This entry lists two keys the PR itself says don't run yet:

  • h3-bf16-h100-sglang-video: "pending, the runner's hub cache is not writable from this account"
  • h3-bf16-mi355x-sglang-video: CUDA image on AMD (see the comment on amd-master.yaml)

Why it matters: The changelog entry triggers result collection for every key it lists, so the sweep that gates /use <run_id> will never be all green while these two are in it.

Fix: List only the validated SKUs now and append a separate entry for the others once they pass:

Suggested change
- h3-bf16-h100-sglang-video
- h3-bf16-mi355x-sglang-video
- h3-bf16-h200-sglang-video
- h3-bf16-b200-sglang-video

(Delete the matching lines above as well, so the entry ends up with just the two validated keys.)

@@ -1091,6 +1099,50 @@ def _agentic_entries(
return entries


def _video_entries(

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 Count Report 📈

  • Total Lines: 1828
  • Base Lines: 1766
  • Change: +62 lines

@claude claude Bot left a comment

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.

Findings marked 🟡 are optional suggestions and need no follow-up push.

Fields.RUNNER.value: runner,
Fields.SPEC.value: spec,
Fields.GPUS.value: gpus,
Fields.CONC_LIST.value: list(concurrencies),

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.

🔴 Two tools that read every master-config key will crash after merge, because video rows have no conc, isl or tp. They are the infx.matrix.changed command that contributors are told to run, and the scheduled Klaud planner. Video rows carry conc-list (generate.py:1133), but these readers index the fixed-sequence fields directly. Fix: in each reader, handle scenario-type == "video-generation" (read conc-list, skip isl/tp) or skip those rows, covering the 4 sites listed. Same pattern at 4 sites (matrix/changed.py:32, matrix/changed.py:102, klaud/main.py:116, matrix/plan.py:133).

Why this was flagged

changed.head_matrix (changed.py:37-46) calls generate_config_matrix([key], ...) for every key in the master configs. That now includes h3-bf16-*-sglang-video, whose rows _video_entries builds with conc-list and no conc. _points then evaluates row["conc"] at changed.py:32, outside its ValueError guard, and raises KeyError. So python -m infx.matrix.changed --base origin/main dies for every PR. CONTRIBUTING.md, the PR template and claude.yml all require running that command. Once that is fixed, _label (changed.py:102) still reads point['tp']. klaud-plan.yml runs python -m infx.klaud plan every 2 hours. plan calls live_families, which generates every master key and reads entry["isl"] at klaud/main.py:116. Only ValueError from generation is caught there, so a KeyError aborts the planner. A future append-only entry on a video key also fails in plan.py:133-138 with "invalid concurrency value: None". On base, every generated row has conc/isl/tp, so none of these readers fail. test_changed.py uses synthetic configs and does not catch this.

Verification: Trigger: anyone runs python -m infx.matrix.changed with the default master configs once this PR is merged. _video_entries builds rows with Fields.CONC_LIST.value: list(concurrencies) (generate.py:1133) and no conc key. The recipe_fingerprint call sits inside try/except ValueError. The next line (changed.py:32) is outside the try, and it raises KeyError: 'conc' on every video row.

Comment thread .github/workflows/benchmark-tmpl.yml Outdated
if: ${{ always() && inputs.scenario-type == 'video-generation' && env.RESULT_FILENAME != '' }}
uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
with:
name: h3-results-${{ github.run_id }}-${{ github.run_attempt }}

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.

🔴 When one run has two or more video rows (all four -sglang-video keys share a changelog entry), every SKU job after the first fails to upload results. The artifact name h3-results-${{ github.run_id }}-${{ github.run_attempt }} holds no row identity, and upload-artifact v4+ rejects a name already used in the run. Fix: give each matrix job its own name, e.g. h3-results-${{ env.RESULT_FILENAME }} plus the runner, matching the bmk_${{ env.RESULT_FILENAME }} pattern, and update the app ingester if it reads the old name.

Why this was flagged

e2e-tests.yml test-sweep-video (e2e-tests.yml:506) fans out one benchmark-tmpl.yml job per video row. The perf-changelog entry lists h3-bf16-h200/b200/h100/mi355x-sglang-video, so a merge or changelog-driven run creates four jobs in the same workflow run. Each job reaches the Upload video results step at benchmark-tmpl.yml:339-346 with the same name, h3-results-<run_id>-<run_attempt>. actions/upload-artifact v7 does not allow a duplicate artifact name within a run. The step has no continue-on-error, so every job after the first fails and its h3-results bundle is lost, even after a valid benchmark. The other artifacts in this template avoid this by putting env.RESULT_FILENAME in the name. The PR's validation ran a single row per run, so it never hit the collision.

Verification: Trigger: any e2e run whose matrix has two or more video-generation rows. In each job, the new step at benchmark-tmpl.yml:339-346 uploads with name: h3-results-${{ github.run_id }}-${{ github.run_attempt }}. The step uses actions/upload-artifact v7.0.1, has no overwrite: true and no continue-on-error. Every later SKU job fails at this step, so its job is marked failed.

Comment on lines +429 to +431
scenario = "agentic"
elif result.get("scenario-type") == "video-generation":
scenario = "video"

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.

🔴 Labeled PR sweeps for the new H3 changelog entry run no video jobs, yet still pass, so nothing is measured or published on merge. plan.py:431 files video rows under the new single_node["video"] bucket. run-sweep.yml only fans out single_node['1k1k'], ['8k1k'] and ['agentic'], so the video bucket is dropped silently. Only e2e-tests.yml got a test-sweep-video job. Fix: give run-sweep.yml a video job that sends single_node.video rows to benchmark-tmpl.yml with scenario-type: video-generation, or fail plan generation when no workflow consumes the bucket.

Why this was flagged

Same-repo PRs that touch inferencex-e2e/perf-changelog.yaml and carry full-sweep-enabled, non-canary-full-sweep-enabled or full-sweep-fail-fast run run-sweep.yml. Its setup job (run-sweep.yml:294-317) calls infx.matrix.plan, and the H3 entry's rows go to final_results["single_node"]["video"] (plan.py:429-433). benchmark_schema --plan accepts that group (benchmark_schema.py, video=group == "video"). The benchmark jobs in run-sweep.yml (lines 483-541) and canary-select (line 348) only read single_node 1k1k, 8k1k and agentic, plus multi_node. The four h3-bf16-*-sglang-video rows therefore schedule zero jobs, and the sweep finishes green. merge-ingest.yml:4 publishes only the reused run-sweep.yml artifacts, so the merged changelog entry gets no H3 results. Before this change these keys had no video rows at all. The diff updated only e2e-tests.yml (test-sweep-video), which external PRs reach via trusted-external-sweep.yml. run-sweep.yml is not in the changed files.

Verification: Trigger: a same-repo PR touches inferencex-e2e/perf-changelog.yaml and carries a sweep label. plan.py:428-434 puts video-generation rows in single_node.video. In run-sweep.yml the sweep jobs only fan out single_node['1k1k'], ['8k1k'] and ['agentic']. So no video job is scheduled, and the run finishes green without measuring any H3 rows.

Comment thread inferencex-e2e/configs/amd-master.yaml Outdated
# (sglang 71de97b with the H3 server-timing patch and the evaluator's media
# dependencies); it is not pullable from a registry.
h3-bf16-mi355x-sglang-video:
image: hongccc/sglang-omni:h3-runtime-71de97b

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.

🔴 After merge, the changelog entry starts an MI355X video job that runs a CUDA-only H3 image on AMD GPUs, so it fails every time. The h3-bf16-mi355x-sglang-video row pins hongccc/sglang-omni:h3-runtime-71de97b, the H200/H100 CUDA runtime. The PR description says MI355X is still blocked on an AMD runtime. Fix: drop the mi355x key from amd-master.yaml and from the perf-changelog config-keys until a ROCm H3 image exists, or pin a ROCm image built for that row.

Why this was flagged

The new perf-changelog.yaml block lists h3-bf16-mi355x-sglang-video, so merging dispatches that row to cluster:mi355x-amds. amd-master.yaml:1740 sets its image to hongccc/sglang-omni:h3-runtime-71de97b. The nvidia-master comment and the PR description both describe this as the H200/H100 CUDA build, staged as a squash on the NVIDIA clusters. The H3 server in run_server.sh then cannot start on MI355X, and the readiness probe times out after 900 s. The result is a failed GPU job and a red changelog run for a SKU the author calls blocked. On the base branch the key does not exist and nothing is dispatched. Nothing in validation checks that the image's vendor matches the runner.

Verification: amd-master.yaml defines h3-bf16-mi355x-sglang-video with image: hongccc/sglang-omni:h3-runtime-71de97b and runner: cluster:mi355x-amds. nvidia-master.yaml uses the same image for the h200 and h100 rows. prepare_amd_runtime.py:12 sets a different, ROCm-based image. The server never becomes ready and the job fails.

Comment on lines +181 to +186
bundle_rc = build_video_bundle(
request.inferencex_results_python, run.workspace, logs,
package=run.workspace.parent / "experimental/video-generation",
output=run.workspace / VIDEO_BUNDLE, result_filename=request.result_filename,
concurrencies=request.conc_list,
) # fmt: skip

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.

🟡 (optional) Every h3-results bundle's ci.json will show null power.producer_git_commit and exporter_image_sha256, so the power provenance the app ingests is lost. finalize_video_results builds the bundle from the fetched logs/power directory. _stage_power_package only copies power-producer-sha.txt and exporter-image.sha256 there later, in the finish_video exit callback. Fix: copy both workspace provenance files into logs/power before build_video_bundle runs. Alternatively, pass their workspace paths to srt-finalize, so ci.json always carries the producer SHA and exporter digest.

Why this was flagged

In run_video (infx/launch/drivers/srt/init.py), the collect stage calls collect.finalize_video_results(run, logs, checkout.commit) on the freshly fetched fetched/logs. That function calls build_video_bundle at collect.py:181 with --power-dir logs/power. evaluator/mvp_srt_bundle.py:370-371 reads power_dir / "power-producer-sha.txt" and power_dir / "exporter-image.sha256" into ci.json. Those two files exist only in run.workspace (written by checkout.py:98 and the exporter staging step). _stage_power_package (collect.py:96) copies them into logs/power, but only when the finish_video callback runs at launch exit, after the bundle is already written. As a result, h3-results/ci.json on every native run has "producer_git_commit": null, "exporter_image_sha256": null. This is new code with no base equivalent. No safeguard catches it, because _text() returns None silently for a missing file.

Verification: This happens on every srt-video run: the ci.json provenance fields are always null, and nothing else breaks. Both files are copied into logs/power only by _stage_power_package (collect.py:96-97). It runs after finalize_video_results has already written h3-results/ci.json, and the bundle is never rebuilt.

assert {"conc", "isl", "model-prefix"} <= scalar_reads
[row] = generate_config_matrix([H200], masters, runners, root=ROOT)
nested = {field: row[field] for field in scalar_reads if isinstance(row.get(field), (list, dict))}
assert nested == {}

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.

🟡 (optional) Several new tests pin checked-in config or grep a workflow file, which AGENTS.md's Test quality rules forbid, so they break when a valid SKU or recipe is added. test_every_video_config_targets_one_cluster_and_the_same_frozen_workload (line 45) asserts the exact list of four keys. test_video_rows_keep_scalars_where_the_benchmark_template_reads_config_fields (line 73) regex-reads benchmark-tmpl.yml. test_the_h200_video_config_generates_one_job_for_every_concurrency (line 34) pins runner, gpus, conc-list and spec. Fix: rebuild these tests on controlled master-config dicts passed to generate_config_matrix/validation, and delete the source-reading test.

Why this was flagged

AGENTS.md Test quality forbids reading source text (rule 1) and pinning checked-in config, including the count of SKUs (rule 4). It also says a test must not fail when a valid recipe or SKU is added. test_video_matrix.py:47-50 asserts that the sorted video keys equal exactly four names, so adding a fifth valid SKU fails the test. test_video_matrix.py:76-78 opens .github/workflows/benchmark-tmpl.yml and runs re.findall on its text. Lines 36-41 assert master-config values from nvidia-master.yaml, such as cluster:h200-dgxc, [1, 2, 4] and h3-serving-2gpu.json. A config bump or adding a SKU makes these fail with no change in behavior.

Verification: In inferencex-e2e/infx/tests/matrix/test_video_matrix.py, lines 47-50 assert the four video keys exactly. Adding a fifth valid video SKU to a master config fails this test. Line 76 reads .github/workflows/benchmark-tmpl.yml with .read_text(), which rule 1 (line 90) of AGENTS.md forbids. AGENTS.md is not modified by this PR, so its rules apply.

INFMAX_CONTAINER_WORKSPACE="$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)"
export INFMAX_CONTAINER_WORKSPACE
source "$INFMAX_CONTAINER_WORKSPACE/benchmarks/check_env.sh"
check_env_vars H3_RESULT_DIR H3_SPEC_PATH H3_PACKAGE_ROOT H3_SERVER_SERVICE H3_SERVER_PORT \

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.

🟡 nit (optional): New files open with file or section header comments, which AGENTS.md's Code comments rule bans. sweep:^# (Custom-benchmark H3|GPU-owning H3|MiniMax-H3 video serving|Video-generation benchmarks) Example: run_client.sh:2-4 and run_server.sh:2-4. Fix: delete these headers and keep only short technical line comments. [also at: inferencex-e2e/benchmarks/single_node/srt-slurm-recipes/h3/sglang/h200-bf16/serving-client.yaml:8 - nit: the new recipes, shims and master-config blocks open with file or section header comments, which AGENTS.md forbids. sweep:^# (MiniMax-H3 video serving|Custom-benchmark H3 client|GPU-owning H3 diffusion|Video-generation benchmarks).]

Why this was flagged

AGENTS.md 'Code comments (mandatory)' applies to recipes, configs, scripts and source: 'Never add broad, overarching comments at the top of a file or above a recipe, override, or section.' run_client.sh:2-4 and run_server.sh:2-4 describe what each script is. Each of the four serving-client.yaml recipes opens with an 8-line description block. amd-master.yaml and nvidia-master.yaml add 8-line section headers above the h3 keys. These comments describe the files rather than explain specific lines.

Verification: AGENTS.md is not changed by this PR, and its lines 60-64 say: "No header comments. Never add broad, overarching comments at the top of a file or above a recipe, override, or section." inferencex-e2e/benchmarks/h3/run_client.sh lines 2-4 and run_server.sh lines 2-4 describe the whole script. Nothing breaks at runtime.

@edwingao28 edwingao28 added the full-sweep-fail-fast Full sweep with canary gate; first failure cancels the rest of that matrix (recommended) label Oct 18, 2026
@edwingao28 edwingao28 changed the title [VideoGenX] run H3 video generation through the native srt-slurm launcher / 通过原生 srt-slurm launcher 运行 H3 视频生成 [VideoGenX] run H3 through native sweeps with measured power Oct 18, 2026
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:将 PR #2916 的 H3 serving spec 原样复制为测试 fixture。
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:新增 SGLang 与 vLLM-Omni 共用的 MiniMax-H3 workload spec 及 loader;生成的 plan 及其 plan_sha256 与 #2916 runner 一致。
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:将 PR #2916 的 H3 serving spec 原样复制为测试 fixture。
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:新增 SGLang 与 vLLM-Omni 共用的 MiniMax-H3 workload spec 及 loader;生成的 plan 及其 plan_sha256 与 #2916 runner 一致。
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:将 PR #2916 的 H3 serving spec 原样复制为测试 fixture。
edwingao28 added a commit that referenced this pull request Oct 18, 2026
中文:新增 SGLang 与 vLLM-Omni 共用的 MiniMax-H3 workload spec 及 loader;生成的 plan 及其 plan_sha256 与 #2916 runner 一致。
@github-actions

Copy link
Copy Markdown
Contributor

This branch has not been deployed

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

Labels

full-sweep-fail-fast Full sweep with canary gate; first failure cancels the rest of that matrix (recommended)

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

2 participants