Repository navigation
[CI] Remove GLM-5.2 spec decode CI tests - #16630
Merged
wangxiyuan merged 1 commit intoSep 16, 2026
Merged
Conversation
Signed-off-by: w30075777 <wujunjie39@h-partners.com>
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request streamlines the CI process by removing specific end-to-end tests for GLM-5.2 speculative decoding. These tests were identified as redundant due to existing coverage in the nightly test suite, allowing for a cleaner repository and reduced CI overhead. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Test][Misc] Remove GLM-5.2 speculative decoding end-to-end testsSuggested PR Summary:
### What this PR does / why we need it?
This pull request removes the end-to-end tests for GLM-5.2 speculative decoding (DSpark and MTP) on Ascend NPU hardware.
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
This is a test-only cleanup PR. No new tests were added.I have no review comments to address, and therefore no additional feedback to provide.
Contributor
|
👋 Hi! Thank you for contributing to the vLLM Ascend project. The following points will speed up your PR merge:
If CI fails, you can run linting and testing checks locally according Contributing and Testing. |
lilinsiman
approved these changes
Sep 16, 2026
wangxiyuan
approved these changes
Sep 16, 2026
tanjiangshan
added a commit
to tanjiangshan/vllm-ascend
that referenced
this pull request
Sep 16, 2026
Register the nightly perf coverage for the PROLOG_V3 fused decode route instead of a PR-level e2e case, following vllm-project#16630 (GLM-5.2 spec decode PR-level CI tests are removed as nightly-covered): - register sfa_prolog_v3 in the coverage taxonomy ALLOWED_VALUES - map enable_sfa_prolog_v3 in the nightly _FEATURE_CONFIGS - add nightly perf config GLM-5.2-W4A8C8-SFA-PrologV3.yaml and register the glm-5.2-w4a8c8-sfa-prolog-v3 job for a3 in nightly_config.yaml The originally included eight_card PR e2e case (test_glm_5_2_mtp_acceptance_tp8_sfa_prolog_v3) is dropped along with the upstream removal of tests/e2e/pull_request/eight_card/test_glm5_2.py. Signed-off-by: huamus <1943805462@qq.com>
tanjiangshan
added a commit
to tanjiangshan/vllm-ascend
that referenced
this pull request
Sep 16, 2026
Add full-forward UT coverage for the enable_sfa_prolog_v3 decode path, mirroring test_sfa_nope_forward.py: the real AscendSFAImpl.forward runs on tiny dimensions with the NPU ops replaced by pure-PyTorch reference implementations, and the output is checked against an independently computed expectation. - decode-only step: the PROLOG_V3 fused op is invoked once with the expected wiring (int64 cache indices, fused weight products, MXFP8 quant branch, kv_cache_quant_mode), raw hidden states are handed to the indexer k path, and end-to-end numerics are verified against the reference chain; - prefill step: falls back to the NATIVE chain (fused op untouched, exec_kv writes the caches through the int64-cached slot mapping); - two forwards on one metadata share a single int64 slot conversion: both fused calls alias the cached copy storage (the .view(-1) inside the preprocess is a zero-copy view, never a re-cast). This replaces the nightly-only coverage originally wired in this PR: per vllm-project#16630 the GLM-5.2 spec-decode e2e jobs are no longer run at PR level, and this UT exercises the actual forward execution path on every push instead. Signed-off-by: huamus <1943805462@qq.com>
tanjiangshan
added a commit
to tanjiangshan/vllm-ascend
that referenced
this pull request
Sep 16, 2026
Add full-forward UT coverage for the enable_sfa_prolog_v3 decode path, mirroring test_sfa_nope_forward.py: the real AscendSFAImpl.forward runs on tiny dimensions with the NPU ops replaced by pure-PyTorch reference implementations, and the output is checked against an independently computed expectation. - decode-only step: the PROLOG_V3 fused op is invoked once with the expected wiring (int64 cache indices, fused weight products, MXFP8 quant branch, kv_cache_quant_mode), raw hidden states are handed to the indexer k path, and end-to-end numerics are verified against the reference chain; - prefill step: falls back to the NATIVE chain (fused op untouched, exec_kv writes the caches through the int64-cached slot mapping); - two forwards on one metadata share a single int64 slot conversion: both fused calls alias the cached copy storage (the .view(-1) inside the preprocess is a zero-copy view, never a re-cast). This replaces the nightly-only coverage originally wired in this PR: per vllm-project#16630 the GLM-5.2 spec-decode e2e jobs are no longer run at PR level, and this UT exercises the actual forward execution path on every push instead. Signed-off-by: huamus <1943805462@qq.com>
ZT-AIA
pushed a commit
that referenced
this pull request
Sep 18, 2026
…ed decode for non-PD serving (#16328) ### What this PR does / why we need it? GLM5.2 SFA profiling shows several removable ops in the K processing path of every layer. This PR removes them and makes PROLOG_V3 the default fused preprocessing for quantized SFA layers in every deployment: 1. **Per-step int64 slot cast**: `exec_kv` re-cast the shared slot mapping to int64 for `npu_kv_rmsnorm_rope_cache` in every layer, although all layers of a scheduling step receive the same int32 slot tensor. The conversion is now cached on the attention metadata (`_int64_kv_slots`), so one Cast kernel runs per step instead of one per layer (~5us x num_layers per step). The PROLOG_V3 fused preprocess reuses the same cached conversion for its int64 cache indices. 2. **Duplicate indexer GEMM**: the indexer's k path (`forward_k`) and top-k stage (`forward`) both ran the same `wk_weights_proj` GEMM (`[tokens, hidden] x [160, hidden]`) on the same hidden states, once for the indexer K and once for the lightning-indexer weights. `forward_k` now returns the non-K tail of the GEMM output and `forward` reuses it, removing one GEMM plus its slice copy per indexer layer per step (falling back to the GEMM only when the two stages are handed different tensors). 3. **Redundant `.contiguous()` copies**: `npu_rms_norm` returns a contiguous tensor so the copy before the C8 block-quant view was discarded, and `torch.cat` already allocates contiguous outputs for the sparse-attention query concat. 4. **PROLOG_V3 by default, no new switch**: PROLOG_V3 (the `npu_mla_prolog_v3` single fused op covering qkv proj + norm + rope + q up-proj + C8 quantize/pack + direct cache write) was previously gated on `is_kv_consumer`, i.e. only PD-disaggregated decode workers could take it. It is now the default fused preprocessing for quantized SFA layers in every deployment (plain serving, PD KV producers and KV consumers) and serves every attention state: prefill and decode steps both take the fused path (the per-step attention-state fallback to NATIVE is gone; only MLAPO keeps its token-count limit). Switch convergence: - `enable_dsa_cp` is the prefill/P-node route selector: it routes to `AscendSFADSACPImpl`, which unconditionally disables fused preprocessing, so the two are mutually exclusive by construction (dsa_cp on => prolog off, dsa_cp off => prolog on); - the C8 switches (`enable_sparse_sfa_c8` / `enable_sparse_li_c8`) only select the KV cache layout and are orthogonal to this choice; W8A8Dynamic layers no longer require `enable_sparse_sfa_c8` to take the fused path; - unquantized layers keep the NATIVE chain outside KV consumers because the unquantized weight preparation transposes `fused_qkv_a_proj.weight` in place, which the NATIVE fallback still consumes; - `dispose_layer` stays gated on `is_kv_consumer` so producers and plain-serving workers keep the fallback weights (the cost is the extra PROLOG_V3 weight copies: memory, not correctness). ### Does this PR introduce _any_ user-facing change? Yes, a default behavior change: quantized (W8A8Dynamic / W8A8MXFP8) SFA deployments now take the PROLOG_V3 fused preprocessing for both prefill and decode steps by default, without any additional-config option. Deployments on `enable_dsa_cp` (prefill/P-node CP route) and unquantized (bf16) layers are unaffected. The default trades extra NPU weight memory (the retained qkv_a/q_b fallback weights on producers and plain-serving workers) for kernel savings. ### How was this patch tested? - Unit tests added/updated in `tests/ut/attention/test_sfa_v1.py`: - per-step int64 slot caching (passthrough / convert-and-cache / re-convert on new step) and `exec_kv` reusing the cached slots across layers; - single `wk_weights_proj` invocation across the indexer k path and top-k stage (plus the fallback recomputation when the tensors differ); - PROLOG_V3 routing matrix for the default gate (non-PD W8A8Dynamic with and without C8 -> PROLOG_V3; non-PD MXFP8 -> PROLOG_V3; non-PD unquantized -> NATIVE; KV producer quantized -> PROLOG_V3, unquantized -> NATIVE); - weight-disposal guard confirming non-consumer workers keep the fallback weights. - `uvx ruff==0.14.0 check` and `format --check` on all touched Python files. - CI cpu-ut green (4431+ tests). **End-to-end A/B benchmark: DSA-CP route (main) vs PROLOG_V3 route (this PR)** (Atlas 800 A3, 16x 910B, CANN 9.1.0, vllm 0.28.0, GLM-5.2-w4a8c8, DP2xTP8 + EP): The comparison is between the two decode preprocessing routes, each in its best usable configuration. `enable_dsa_cp` requires SP-MoE and unconditionally disables the fused preprocessing path, so the two options are mutually exclusive by construction; everything else is identical on both sides: | item | base (main `125924bb2`) | PR (`cb525a869`) | |---|---|---| | **decode preprocessing route** | `enable_dsa_cp=true` | PROLOG_V3 route (now the default; measured with the earlier opt-in build of this PR) | | SP-MoE / sequence parallelism (`VLLM_ASCEND_ENABLE_FLASHCOMM1=1`; auto-enabled by DSA-CP on the base side) | on | on | | `enable_sparse_sfa_c8` + `enable_sparse_li_c8` | on | on | | `enable_balance_scheduling`, `enable_fused_mc2=0` | on | on | | `--enable-expert-parallel` (EP), DP2xTP8 | on | on | | MTP speculative decoding (`deepseek_mtp`, num_speculative_tokens=3, enforce_eager) | on | on | | cudagraph `FULL_DECODE_ONLY`, `--quantization ascend` | on | on | | `multistream_overlap_shared_expert` | off | off (incompatible with FlashComm1 on DP>1, see #16446) | Both sides verified via serve logs: no "Disabling DSA-CP" / sp-MoE active on the base side, `MlaPrologV3` kernels present in the PR-side profile only. Workload: GSM8K test full 1319 prompts, ais-bench stream mode, concurrency 8, temperature 0. | metric | base (DSA-CP route) | PR (PROLOG_V3 route) | delta | |---|---|---|---| | TPOT avg | 27.9 ms | **22.9 ms** | **-17.9%** | | TPOT median | 27.8 ms | 22.8 ms | -18.0% | | E2EL avg | 7,181.6 ms | **5,885.0 ms** | -18.1% | | TTFT avg | 459.9 ms | 395.8 ms | -13.9% | | per-request output throughput | 33.73 tok/s | **40.91 tok/s** | +21.3% | | request throughput (aggregate) | 1.1117 req/s | 1.3564 req/s | +22.0% | | failed requests | 0/1319 | 0/1319 | = | Kernel-level verification (rank0 profile of a 500-token decode request): | kernel | base (DSA-CP) | PR (PROLOG_V3) | note | |---|---|---|---| | MlaPrologV3 | 0 | **13,369** | fused decode path taken over | | InterleaveRope | 28,980 | **1,156** | -96%: NATIVE rope chain leaves decode | | DynamicBlockQuant | 14,490 | **578** | -96%: c8 quant chain leaves decode | | ScatterNdUpdate | 21,035 | 6,967 | -67% | | Slice | 31,556 | 12,211 | -61% count | | QuantBatchMatmulV3 | 73,652 (1,629 ms) | 44,207 (835 ms) | -40% count / -49% time | | KvQuantSparseFlashAttention | 14,190 | 13,703 | ~same (attention body) | | **total decode kernels** | **711,070** | **479,326** | **-32.6%** | **E2E coverage for the PROLOG_V3 default route**: - Following #16630 (GLM-5.2 spec-decode PR-level CI tests are removed as nightly-covered), the fused path is exercised by the nightly-covered GLM-5.2 spec-decode e2e suites together with the one-shot hardware verification below; no dedicated per-push full-forward UT is carried in this PR. - Verified on real hardware (Atlas 800 A3, 8x 910B, CANN 9.1.0, vllm-ascend 0.28.0): the PROLOG_V3-route e2e case PASSED (acceptance length within 3.06 +/- 8%) when run as the eight_card job on branch `perf/glm-kpath-fusion-28` and in the E2E CI run of commit `28e9e11` (all 24 jobs green), confirming the fused route serves plain (non-PD) decode correctly under the full serving stack. - Note: serving the w4a8c8 checkpoint on torch_npu 2.10 additionally requires a one-word out-of-tree fix in `vllm_ascend/quantization/methods/w4a8/w4a8.py` (`.sum(axis=1)` -> `.sum(dim=1)`; the `axis` kwarg comes from #13713 and torch_npu's `reduce_sum` rejects it; fixed on main by #16413). - vLLM main: vllm-project/vllm@84030bb --------- Signed-off-by: huamus <1943805462@qq.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
What this PR does / why we need it?
Remove GLM-5.2 spec decode CI tests covered by nightly tests.
Does this PR introduce any user-facing change?
No.
How was this patch tested?
Nightly test passed.