-
Notifications
You must be signed in to change notification settings - Fork 156
[NV] update minimaxm2.5 fp4 b300 vllm #1107
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
Changes from all commits
fc652ef
7f12a85
14bf54d
293143f
50a129c
fdd33da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1684,3 +1684,9 @@ | |
| description: | ||
| - "Add VLLM_FLOAT32_MATMUL_PRECISION=high, remove VLLM_FLASHINFER_ALLREDUCE_BACKEND=mnnvl" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1106 | ||
|
|
||
| - config-keys: | ||
| - minimaxm2.5-fp4-b300-vllm | ||
| description: | ||
| - "Add VLLM_FLOAT32_MATMUL_PRECISION=high, update search space concurrency ranges" | ||
| pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/1107 | ||
|
Comment on lines
+1689
to
+1692
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The new perf-changelog.yaml entry uses an unresolved placeholder 'pull/XXX' in the pr-link field; since this PR has been assigned #1107, the link should be updated to 'pull/1107' before merging so the changelog entry can be traced back to this PR. Extended reasoning...The new changelog entry added by this PR at perf-changelog.yaml:1654 contains 'pr-link: https://github.com/SemiAnalysisAI/InferenceX/pull/XXX'. The PR being reviewed is #1107, so the correct link is '#1107'. This means the changelog entry, which is meant to serve as a traceable record of what changed and when, cannot be linked back to this PR after merge. The specific code path is simple: the perf-changelog.yaml file is a human-maintained audit trail mapping config-keys to the PR that introduced each change. The entry for 'minimaxm2.5-fp4-b300-vllm' documents 'Add VLLM_FLOAT32_MATMUL_PRECISION=high, update search space concurrency ranges' but the pr-link placeholder prevents future readers from finding the originating PR. The refutation argument notes that XXX/XXXX placeholders appear in approximately 22 other entries in the file, making it an 'accepted pattern.' However, accepted pattern does not mean correct behavior — those entries are also missing proper PR links and represent the same documentation debt. The PR being [WIP] further explains why the link was left as a placeholder during development, but the expectation should be that it is resolved before merging (i.e., right now, since the PR number is already known). The impact is limited to documentation quality. No runtime behavior, benchmark configurations, or benchmark scripts are affected. However, an XXX link in the merged changelog permanently obscures which PR introduced this change, making historical archaeology harder for maintainers. The changelog's value is precisely in its traceability. The fix is a one-line change: replace 'pull/XXX' with 'pull/1107' in the last line of the new entry at perf-changelog.yaml:1654. This is a nit-level issue — it should be fixed before merging but does not block functionality. |
||
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.
can u update the vllm recipe?