Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Oseltamivir merged 1 commit into from
Apr 25, 2026
+9 −1
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions perf-changelog.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1786,3 +1786,11 @@
- "Launch args match the provided vllm serve command, including FP4 indexer cache, FULL_AND_PIECEWISE cudagraph config, and max-num-batched-tokens 2048"
- "1k1k uses --max-model-len 4096; 8k1k uses the workflow-provided benchmark context length"
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1144

- 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"

Check failure on line 1796 in perf-changelog.yaml

View check run for this annotation

Claude / Claude Code Review

Missing pr-link field in new changelog entry will fail validation 

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/Infe
Comment on lines +1789 to +1796
Copy link
Copy Markdown
Contributor

@claude claude Bot Apr 25, 2026

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.

2 changes: 1 addition & 1 deletion runners/launch_mi355x-amds.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ else
LOCK_FILE="${SQUASH_FILE}.lock"

set -x
salloc --partition=$PARTITION --gres=gpu:$TP --exclusive --cpus-per-task=128 --time=180 --no-shell --job-name="$RUNNER_NAME"
salloc --partition=$PARTITION --gres=gpu:$TP --exclusive --cpus-per-task=128 --time=300 --no-shell --job-name="$RUNNER_NAME"
JOB_ID=$(squeue --name="$RUNNER_NAME" -h -o %A | head -n1)

srun --jobid=$JOB_ID bash -c "docker stop \$(docker ps -a -q)"
Expand Down
Loading