Skip to content

Bump MI355X SLURM time-limit to 300m; retrigger dsv4-fp8-mi355x-sglang#1147

Merged
Oseltamivir merged 1 commit intomainfrom
dsv4-mi355x-slurm-time
Apr 25, 2026
Merged

Bump MI355X SLURM time-limit to 300m; retrigger dsv4-fp8-mi355x-sglang#1147
Oseltamivir merged 1 commit intomainfrom
dsv4-mi355x-slurm-time

Conversation

@Oseltamivir
Copy link
Copy Markdown
Collaborator

Summary

  • DSv4-Pro on MI355X was hitting STEP CANCELLED DUE TO TIME LIMIT at the 3h SLURM cap (e.g. STEP 12903.2 ON mia1-p01-g19 CANCELLED AT 2026-04-25T02:52:29 DUE TO TIME LIMIT). The combination of ~30min MoE JIT compile plus slow torch-fallback kernels (SGLANG_HACK_FLASHMLA_BACKEND=torch and the SGLANG_OPT_*=false set from sgl-project/sglang#23608) pushes 8k1k high-concurrency runs past 180m.
  • Bump runners/launch_mi355x-amds.sh:189 --time=180--time=300. Matches the GH Actions outer timeout-minutes: 300 in benchmark-tmpl.yml so we don't get cut off by the runner SLURM step before the workflow cap.
  • Add perf-changelog.yaml entry against dsv4-fp8-mi355x-sglang to retrigger the benchmark.

For context, MI325X already uses --time=480 for similar reasons (slow large-model runs). 300m is the more conservative bump that still makes the SLURM cap match the outer GH cap.

Test plan

  • CI re-runs dsv4-fp8-mi355x-sglang 8k1k high-concurrency configs without the time-limit cancel
  • Other MI355X configs (e.g. dsr1-*-mi355x-*) continue to pass — they were comfortably under 180m before, so a higher cap is a no-op for them

🤖 Generated with Claude Code

DSv4-Pro on MI355X was hitting `STEP CANCELLED DUE TO TIME LIMIT` at the
3h SLURM cap. ~30min MoE JIT compile plus slow torch-fallback kernels
(SGLANG_HACK_FLASHMLA_BACKEND=torch and the SGLANG_OPT_*=false set from
sgl-project/sglang#23608) push the 8k1k high-concurrency runs past 180m.

Bumping --time=180 -> --time=300 in runners/launch_mi355x-amds.sh. 300
matches the GH Actions outer timeout-minutes in benchmark-tmpl.yml so we
don't get cut off by the runner SLURM step before the workflow cap.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@github-actions
Copy link
Copy Markdown
Contributor

Thanks for the contribution! For vLLM & SGLang, please ensure that your recipes is similar to the official vLLM recipes and/or the SGLang cookbook

If it is not, please create a PR first before we can merge your PR into the master branch. Let's ensure that the documentation is first class such that the entire ML community can benefit from your hard work! Thank you

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. If re-running failed jobs is attempted, PR authors are responsible for ensuring it passes. See GitHub's docs on re-running failed jobs: https://docs.github.com/en/actions/how-tos/manage-workflow-runs/re-run-workflows-and-jobs#re-running-failed-jobs-in-a-workflow

As a rule of thumb, generally, PR authors should request a review & get a PR approval from the respective companies' CODEOWNERS before requesting a review from core maintainers.

If additional help is needed, PR authors can reach out to core maintainers over Slack.

@Oseltamivir Oseltamivir merged commit d40be91 into main Apr 25, 2026
6 checks passed
@Oseltamivir Oseltamivir deleted the dsv4-mi355x-slurm-time branch April 25, 2026 03:10
Comment thread perf-changelog.yaml
Comment on lines +1789 to +1796

- config-keys:
- dsv4-fp8-mi355x-sglang
description:
- "Bump MI355X SLURM allocation from --time=180 to --time=300 in runners/launch_mi355x-amds.sh"
- "DSv4-Pro on MI355X exceeded the 3h cap (STEP CANCELLED DUE TO TIME LIMIT) due to ~30min MoE JIT compile plus slow torch-fallback kernels (SGLANG_HACK_FLASHMLA_BACKEND=torch et al.) from sgl-project/sglang#23608"
- "300 minutes matches the GH Actions outer timeout-minutes cap in benchmark-tmpl.yml"
- "Retriggering dsv4-fp8-mi355x-sglang"
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.

🔴 The new changelog entry for dsv4-fp8-mi355x-sglang at perf-changelog.yaml:1790-1796 is missing the required pr-link field. The pydantic ChangelogEntry model in utils/matrix_logic/validation.py:338-345 declares pr_link as required with no default and uses extra="forbid", so utils/process_changelog.py will raise a ValidationError when this PR's changelog is processed, breaking the run-sweep CI this PR exists to retrigger. Fix by adding pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1147 after the description block.

Extended reasoning...

What the bug is

The changelog entry added by this PR at perf-changelog.yaml:1789-1796 registers a performance-relevant change against dsv4-fp8-mi355x-sglang but omits the pr-link field. Every one of the other 204 entries in this file supplies a pr-link line, even when the PR number is not yet known (entries use placeholders like pull/XXX, pull/XXXX, or pull/TBD). Omitting it entirely is a first.

Why this breaks validation

In utils/matrix_logic/validation.py:338-345:

class ChangelogEntry(BaseModel):
    model_config = ConfigDict(extra="forbid", populate_by_name=True)

    config_keys: list[str] = Field(alias="config-keys", min_length=1)
    description: list[str] = Field(min_length=1)
    pr_link: str = Field(alias="pr-link")
    evals_only: bool = Field(alias="evals-only", default=False)

pr_link has no default value, so pydantic treats it as a required field. When process_changelog.py calls ChangelogEntry.model_validate(entry_data) on the added entry, pydantic will raise:

ValidationError: 1 validation error for ChangelogEntry
pr-link: Field required [type=missing, loc=('pr-link',), ...]

The code path that triggers it

.github/workflows/run-sweep.yml invokes utils/process_changelog.py on every PR that touches perf-changelog.yaml. That script calls get_added_lines(base_ref, head_ref, 'perf-changelog.yaml') and runs ChangelogEntry.model_validate(entry_data) over every added entry. Since the new entry is exactly what was added, validation will fail on this PR.

Impact

This is ironic because the whole point of this PR is to retrigger dsv4-fp8-mi355x-sglang via run-sweep — but run-sweep's changelog-validation step will fail before it can retrigger anything. The --time=300 fix to runners/launch_mi355x-amds.sh is correct and unaffected, but the retrigger won't land until the changelog entry is valid.

Step-by-step proof

  1. Checkout this branch.
  2. Run: python3 -c "import yaml; from utils.matrix_logic.validation import ChangelogEntry; d = yaml.safe_load(open('perf-changelog.yaml')); ChangelogEntry.model_validate(d[-1])"
  3. Observe pydantic error: 1 validation error for ChangelogEntry / pr-link / Field required [type=missing, ...].
  4. Equivalently, CI's run-sweep workflow (.github/workflows/run-sweep.yml) running utils/process_changelog.py on the diff between base and head will hit the same error on this entry.

How to fix

Append a pr-link line at the end of the new entry, matching the style of every other entry, pointing to this PR:

    - "Retriggering dsv4-fp8-mi355x-sglang"
  pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1147

If the PR number is uncertain at this stage, using a placeholder like pull/XXXX (as other entries in the file do) would also satisfy the required-string constraint and let CI proceed.

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.

1 participant