Update minimaxm2.5-fp8-h100-vllm vLLM image to v0.21.0#1399
Conversation
Ref #1154 Co-authored-by: Klaud Cold <Klaud-Cold@users.noreply.github.com>
|
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 single node 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. |
3 similar comments
|
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 single node 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. |
|
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 single node 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. |
|
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 single node 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. |
| - minimaxm2.5-fp8-h100-vllm | ||
| description: | ||
| - "Update vLLM image from v0.20.2 to v0.21.0" | ||
| pr-link: XXX |
There was a problem hiding this comment.
🔴 The new perf-changelog entry sets pr-link: XXX instead of the documented full-URL form https://github.com/SemiAnalysisAI/InferenceX/pull/XXX (see AGENTS.md:123 and every other entry in the file, e.g. line 2495). This breaks the auto-substitution in utils/merge_with_reuse.sh:139 — it does block.replace("/pull/XXX", f"/pull/{pr}"), which will not match the bare XXX token, so the placeholder will be merged to main unchanged. Replace with pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX (or hard-code 1399).
Extended reasoning...
What the bug is
The new entry appended to perf-changelog.yaml at line 2501 reads:
pr-link: XXXEvery other one of the 295 pr-link: entries in this file uses a fully-qualified GitHub URL — e.g. the immediately preceding entry on line 2495 reads pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1271. The documented convention in AGENTS.md:123 is:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXi.e. a full URL with XXX as the placeholder token, not a bare XXX.
Why this matters — merge tooling breaks
This is more than cosmetic because utils/merge_with_reuse.sh exists precisely to auto-resolve perf-changelog.yaml conflicts on merge by substituting the real PR number. The relevant lines are:
- Line 135 detects a PR-side entry whose
pr-linkstill contains theXXXplaceholder - Line 139 performs
block.replace("/pull/XXX", f"/pull/{pr}") - Line 160 asserts
last['pr-link'].endswith('/$PR')after substitution
Because the bare value XXX contains no /pull/XXX substring, step 2 silently does nothing and step 3's assertion then fails, breaking the reuse-sweep merge tool. Even on a normal squash merge that doesn't go through this script, the changelog still lands on main with the meaningless literal pr-link: XXX, which is unparseable as a clickable link and inconsistent with every other entry.
Step-by-step proof
utils/merge_with_reuse.shruns during conflict resolution and, for each new PR-side entry, reads the YAML block.- The check on line 135 — roughly
'XXX' in block['pr-link']— succeeds because'XXX' in 'XXX'is true. - Substitution on line 139 runs:
block_text.replace('/pull/XXX', '/pull/1399'). The source stringpr-link: XXXcontains no/pull/XXXsubstring, so the result is unchanged. - The post-substitution assertion on line 160 —
last['pr-link'].endswith('/1399')— fails because'XXX'.endswith('/1399')is False, and the tool aborts. - If the merge instead proceeds via a plain squash (no conflict path), the file on main literally contains
pr-link: XXX, and any downstream tooling/reader parsing this entry sees an invalid, non-clickable value.
How to fix
Change line 2501 from:
pr-link: XXXto either the template form (matches the convention and lets merge_with_reuse.sh substitute):
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXXor hard-code the actual PR number:
pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1399|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25956498305 |
1 similar comment
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25956498305 |
|
/reuse-sweep-run |
|
see unofficial run visualizer at https://inferencex.semianalysis.com/inference?unofficialRun=25984750514 |
Summary
minimaxm2.5-fp8-h100-vllmfrom v0.20.2 to v0.21.0.Ref #1154
Generated with Claude Code