-
Notifications
You must be signed in to change notification settings - Fork 186
Update gptoss-fp4-b200-vllm vLLM image to v0.20.2 #1333
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.
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
Closed
Closed
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
7afb3e9
$Update gptoss-fp4-b200-vllm vLLM image to v0.20.2\n\nRef #1154\n\nCo…
github-actions[bot] 3ddbf47
Merge branch 'main' into claude/issue-1154-gptoss-fp4-b200-vllm
Oseltamivir 9748d74
Merge branch 'main' into claude/issue-1154-gptoss-fp4-b200-vllm
Oseltamivir File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2343,3 +2343,9 @@ | |
| description: | ||
| - "Add Qwen3.5-397B-A17B FP8 MI355X ATOM benchmark configs with and without MTP" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310 | ||
|
|
||
| - config-keys: | ||
| - gptoss-fp4-b200-vllm | ||
| description: | ||
| - "Update vLLM image from v0.15.1 to v0.20.2" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX | ||
|
Check warning on line 2351 in perf-changelog.yaml
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 The new perf-changelog.yaml entry uses a literal 'XXX' placeholder in its pr-link (
https://github.com/SemiAnalysisAI/InferenceX/pull/XXX) instead of the actual PR number. Since this is PR #1333, the link should be.../pull/1333; as written, anyone following the changelog cross-reference will hit a 404. Fix by replacingXXXwith1333.Extended reasoning...
What the bug is\n\nThe diff adds a new entry to
perf-changelog.yamlat line 2346–2351 documenting the vLLM image bump forgptoss-fp4-b200-vllm. The final field is:\n\nyaml\n pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX\n\n\nTheXXXis a literal placeholder string that was never substituted with the real PR number. This PR is #1333, so the URL should behttps://github.com/SemiAnalysisAI/InferenceX/pull/1333.\n\nWhy existing code doesn'''t prevent it\n\nThe changelog is a free-form YAML document; nothing in the repo validates thatpr-linkURLs point to existing PRs, and YAML syntax accepts the placeholder string just fine. The convention is enforced socially — every other recent entry (lines 2332 →/pull/1304, 2338 →/pull/1305, 2345 →/pull/1310, etc.) uses the real PR number, so this one stands out as an oversight rather than a structural failure.\n\nImpact\n\nThis is metadata only and has no runtime effect — benchmarks, image selection, and CI all ignore the changelog. The harm is purely to changelog traceability: any reader (engineer, release manager, future PR author) who clicks through to understand the context behind the v0.15.1 → v0.20.2 vLLM bump will land on a GitHub 404, since/pull/XXXis not a valid PR. The change history breaks at exactly the point where it'''s being recorded.\n\nHow to fix\n\nReplaceXXXwith1333on line 2351:\n\nyaml\n pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1333\n\n\nStep-by-step proof\n\n1. Open the PR in GitHub — the URL bar shows.../pull/1333, so this PR'''s number is 1333.\n2. View the diff forperf-changelog.yaml. The added block ends with:pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.\n3. Construct the resulting URL:https://github.com/SemiAnalysisAI/InferenceX/pull/XXX.XXXis not a valid integer PR identifier, so GitHub returns a 404 Not Found.\n4. Compare against the entry immediately above (lines 2343–2345):pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1310— a real PR number, which resolves correctly.\n5. The correct value for this entry, by the same convention, is1333. ReplacingXXX→1333restores changelog traceability.\n\nSeverity\n\nNit — the placeholder is clearly an unintentional oversight (a sed/replace step that was skipped) and trivially fixable before merge, but it does not affect runtime behavior.