-
Notifications
You must be signed in to change notification settings - Fork 247
[Klaud Cold] kimik2.5-fp4-b300-vllm: v0.26.0 image, extend conc to 512, add DEP4 arm / 升级 B300 镜像至 v0.26.0,扩展并发,新增 DEP4 #2442
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
base: main
Are you sure you want to change the base?
Changes from all commits
22f4d7f
dfa9591
7746671
129b9b1
80d9ed8
20a5a28
67892db
b13329c
aa76f27
51c8c80
c135460
53081ea
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 |
|---|---|---|
|
|
@@ -406,7 +406,9 @@ else | |
| # MODEL stays as the HF id for the client (--served-model-name, tokenizer); | ||
| # MODEL_PATH is what the server reads weights from. | ||
| MODEL_BASENAME="${MODEL##*/}" | ||
| if [[ " ${STAGED_MODELS[*]} " == *" ${MODEL_BASENAME} "* ]]; then | ||
| if [[ $MODEL_PREFIX == "kimik2.5" && $PRECISION == "fp4" ]]; then | ||
| export MODEL_PATH="${WRITABLE_MODELS_DIR%/}/${MODEL_BASENAME}" | ||
| elif [[ " ${STAGED_MODELS[*]} " == *" ${MODEL_BASENAME} "* ]]; then | ||
| export MODEL_PATH="${HF_HUB_CACHE_MOUNT%/}/${MODEL_BASENAME}" | ||
|
Comment on lines
408
to
412
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 staged-model bypass in Extended reasoning...The new branch added at if [[ $MODEL_PREFIX == "kimik2.5" && $PRECISION == "fp4" ]]; then
export MODEL_PATH="${WRITABLE_MODELS_DIR%/}/${MODEL_BASENAME}"
elif [[ " ${STAGED_MODELS[*]} " == *" ${MODEL_BASENAME} "* ]]; then
export MODEL_PATH="${HF_HUB_CACHE_MOUNT%/}/${MODEL_BASENAME}"
...It's inserted before the
Both route through the same single-node else-branch of Why this matters — concrete proof:
Existing code doesn't prevent this because the new condition was added as a plain prefix/precision predicate rather than being scoped to the specific model basename ( Fix: scope the new branch to the actual model basename (e.g. |
||
| else | ||
| export MODEL_PATH="${WRITABLE_MODELS_DIR%/}/${MODEL_BASENAME}" | ||
|
|
||
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 top-of-file comment in kimik2.5_fp4_b300.sh (lines 3-5) and the matching comment in configs/nvidia-master.yaml (lines 1526-1528) both say the B300 recipe 'reuses the existing Kimi-K2.5 FP4 B200 vLLM recipe as-is until B300-specific tuning is available' — but this PR adds exactly that B300-specific tuning (DP_ATTENTION/EP handling, prefill-schedule-interval, fp8 kv-cache, attention-config, linear-backend, new env vars), so the comment is now stale and misleading. Purely a documentation issue — recommend removing or updating both comments.
Extended reasoning...
What the bug is: The unmodified top-of-file comment in
benchmarks/single_node/fixed_seq_len/kimik2.5_fp4_b300.sh(lines 3-5) states:The identical claim is duplicated in
configs/nvidia-master.yamlat lines 1526-1528, directly above thekimik2.5-fp4-b300-vllmconfig block. Neither comment was touched by this PR, but this PR is precisely what makes them false.Why it's now false: Comparing the new
kimik2.5_fp4_b300.shvllm serveinvocation againstkimik2.5_fp4_b200.sh, the two scripts diverge substantially after this PR:DP_ATTENTIONbranch that switches--tensor-parallel-size/--data-parallel-size, adjusts GPU memory utilization (0.90 -> 0.85), and adds--prefill-schedule-interval 4; B200 has none of this.EP_ARGS->--enable-expert-parallel; absent in B200.--kv-cache-dtype fp8,--attention-config '{\"mla_prefill_backend\":\"FLASHINFER\",...}', and--linear-backend flashinfer_cutlass; none of these exist in B200.--max-cudagraph-capture-sizeis now $((CONC*2)) vs B200's fixed 2048, and--stream-intervalis 32 vs B200's 20.VLLM_USE_V2_MODEL_RUNNER=0,VLLM_FLASHINFER_AUTOTUNE_SKIP_OPS,VLLM_RPC_TIMEOUT; B200 instead setsVLLM_MEMORY_PROFILER_ESTIMATE_CUDAGRAPHS=0, which B300 does not.So the B300 recipe is no longer a copy-as-is of B200 — it is a distinct, B300-tuned recipe, and that tuning is exactly what the comment says doesn't exist yet.
Code path / proof: A reader opening
kimik2.5_fp4_b300.shsees the comment at lines 3-5 claiming 'reuses B200 as-is,' then scrolls a few lines down (post-PR, lines 19-31 and 67-86) and sees DP_ATTENTION branching, EP handling, fp8 kv-cache, attention-config, and linear-backend flags that have no B200 counterpart. The comment directly contradicts the code immediately below it. The same applies to thenvidia-master.yamlcomment at lines 1526-1528, sitting right above thekimik2.5-fp4-b300-vllm:block that this PR also modifies (new nightly image, new DEP4 search-space arm).Why existing code/review doesn't catch this: This is a plain-text comment, not something any linter, test, or CI check inspects — it can only be caught by manual review comparing the comment's claim against the diff.
Impact: Purely a documentation/maintainability issue — nothing breaks functionally. But a future engineer syncing B300 with B200, or auditing why the recipes differ, would be misled into thinking B300 has no tuning beyond a straight copy, when in fact it now has meaningfully different parallelism, quantization, and scheduling logic.
Fix: Remove or rewrite both comments (script header and yaml block comment) to reflect that B300-specific tuning has been added, rather than stating it 'reuses B200 as-is.'