Repository navigation
[Refactor][Ops] Remove causal_conv1d_update_npu - #14620
Merged
realliujiaxu merged 1 commit intoAug 25, 2026
Merged
realliujiaxu merged 1 commit into
realliujiaxu merged 1 commit into
Conversation
Drop the obsolete Triton causal conv1d update kernel and stop overriding vLLM's causal_conv1d_update implementation. Remove the skipped tests that only exercised the deleted operator. Signed-off-by: maoxx241 <maomaoyu870@gmail.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 cleans up the vLLM-Ascend codebase by removing an obsolete Triton-based implementation of the causal convolution update operator. By reverting to the upstream vLLM implementation, the project reduces maintenance overhead and eliminates dead code paths that were previously associated with known issues and skipped tests. 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. 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
|
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. |
![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:
[Ops][Misc] Remove Triton-based causal_conv1d_update implementation and testsSuggested PR Summary:
### What this PR does / why we need it?
This pull request removes the Triton-based implementation of `causal_conv1d_update` (`causal_conv1d_update_npu` and its associated kernel `_causal_conv1d_update_kernel_npu_tiled`) from `vllm_ascend/ops/triton/mamba/causal_conv1d.py`. It also removes the corresponding tests in `test_causal_conv1d.py` and the patching logic in `patch_triton.py`.
Feedback:
An issue was identified in `test_causal_conv1d.py` where `causal_conv1d_fn` is imported from `vllm_ascend.ops.triton.mamba.causal_conv1d`, but it is not defined or exported in that module. This will cause pytest collection to fail with an `ImportError`.
### Does this PR introduce _any_ user-facing change?
No, this is a refactoring/cleanup change that removes unused or broken Triton-based operations.
### How was this patch tested?
The patch should be tested by running the remaining tests in `test_causal_conv1d.py` after fixing the broken import.|
|
||
| from vllm_ascend._310p.ops.causal_conv1d import causal_conv1d_fn as causal_conv1d_fn_ref | ||
| from vllm_ascend._310p.ops.causal_conv1d import causal_conv1d_update as causal_conv1d_update_ref | ||
| from vllm_ascend.ops.triton.mamba.causal_conv1d import PAD_SLOT_ID, causal_conv1d_fn |
Contributor
There was a problem hiding this comment.
The import of causal_conv1d_fn from vllm_ascend.ops.triton.mamba.causal_conv1d will fail with an ImportError because causal_conv1d_fn is not defined or exported in vllm_ascend/ops/triton/mamba/causal_conv1d.py. This will cause pytest collection to fail entirely for this test file, even though the test using it is skipped. Please verify where causal_conv1d_fn should be imported from or remove the unused/broken import.
maoxx241
force-pushed
the
codex/remove-causal-conv1d-update-npu
branch
from
August 20, 2026 08:26
9e979ee to
b5a8f7f
Compare
maoxx241
marked this pull request as ready for review
August 20, 2026 12:10
Contributor
Author
|
/cancel https://github.com/vllm-project/vllm-ascend/actions/runs/32348855329 |
realliujiaxu
approved these changes
Aug 24, 2026
Contributor
Author
|
/rerun Rerun (failed jobs only):
|
yiminghub2024
pushed a commit
to yiminghub2024/vllm-ascend
that referenced
this pull request
Aug 25, 2026
### What this PR does / why we need it? This PR removes the obsolete Ascend Triton causal_conv1d_update_npu implementation. - Delete the Triton kernel and Python wrapper. - Stop monkey-patching the vLLM causal_conv1d_update function, leaving the upstream implementation in place. - Remove the two skipped tests that only exercised the deleted operator. - Keep the shared extract_last_width helper and the existing AscendC and 310P causal-conv paths unchanged. The removed tests documented an overflow case and a probabilistic failure, so they did not provide active coverage for this implementation. ### Does this PR introduce _any_ user-facing change? No public API or configuration change. Internally, vLLM-Ascend no longer replaces the vLLM causal_conv1d_update function with this Triton implementation. ### How was this patch tested? - Full repository checks: bash format.sh ci - Python syntax compilation for the three changed files - Verified there are no remaining references to causal_conv1d_update_npu or its Triton kernel NPU operator tests were not run locally because the operator and its skipped tests are removed by this change. - vLLM version: v0.27.1 - vLLM main: vllm-project/vllm@58d3918 Signed-off-by: maoxx241 <maomaoyu870@gmail.com>
pisceskkk
pushed a commit
to pisceskkk/vllm-ascend
that referenced
this pull request
Aug 26, 2026
### What this PR does / why we need it? This PR removes the obsolete Ascend Triton causal_conv1d_update_npu implementation. - Delete the Triton kernel and Python wrapper. - Stop monkey-patching the vLLM causal_conv1d_update function, leaving the upstream implementation in place. - Remove the two skipped tests that only exercised the deleted operator. - Keep the shared extract_last_width helper and the existing AscendC and 310P causal-conv paths unchanged. The removed tests documented an overflow case and a probabilistic failure, so they did not provide active coverage for this implementation. ### Does this PR introduce _any_ user-facing change? No public API or configuration change. Internally, vLLM-Ascend no longer replaces the vLLM causal_conv1d_update function with this Triton implementation. ### How was this patch tested? - Full repository checks: bash format.sh ci - Python syntax compilation for the three changed files - Verified there are no remaining references to causal_conv1d_update_npu or its Triton kernel NPU operator tests were not run locally because the operator and its skipped tests are removed by this change. - vLLM version: v0.27.1 - vLLM main: vllm-project/vllm@58d3918 Signed-off-by: maoxx241 <maomaoyu870@gmail.com> Signed-off-by: QiuChunshuo <qiuchunshuo@huawei.com>
frankie-ys
pushed a commit
to Csrayz/vllm-ascend
that referenced
this pull request
Aug 26, 2026
### What this PR does / why we need it? This PR removes the obsolete Ascend Triton causal_conv1d_update_npu implementation. - Delete the Triton kernel and Python wrapper. - Stop monkey-patching the vLLM causal_conv1d_update function, leaving the upstream implementation in place. - Remove the two skipped tests that only exercised the deleted operator. - Keep the shared extract_last_width helper and the existing AscendC and 310P causal-conv paths unchanged. The removed tests documented an overflow case and a probabilistic failure, so they did not provide active coverage for this implementation. ### Does this PR introduce _any_ user-facing change? No public API or configuration change. Internally, vLLM-Ascend no longer replaces the vLLM causal_conv1d_update function with this Triton implementation. ### How was this patch tested? - Full repository checks: bash format.sh ci - Python syntax compilation for the three changed files - Verified there are no remaining references to causal_conv1d_update_npu or its Triton kernel NPU operator tests were not run locally because the operator and its skipped tests are removed by this change. - vLLM version: v0.27.1 - vLLM main: vllm-project/vllm@58d3918 Signed-off-by: maoxx241 <maomaoyu870@gmail.com>
yiminghub2024
added a commit
to yiminghub2024/vllm-ascend
that referenced
this pull request
Aug 27, 2026
- drop the dead npu_kda_causal_conv1d_triton bind block; causal_conv1d_update_npu was removed upstream in vllm-project#14620, so the block always fell through to the except branch and printed to stdout on every worker start. GLM KDA already binds causal_conv1d_update earlier in the same file. - mark the optional vllm.models.glm5next KDA import as import-not-found - assert kv_a_layernorm in _exec_kv_mla_nope, matching the sibling MLA paths - guard shared_head before rebinding the MTP draft head Signed-off-by: yiminghub2024 <482890@qq.com> Co-authored-by: Cursor <cursoragent@cursor.com>
yiminghub2024
added a commit
to yiminghub2024/vllm-ascend
that referenced
this pull request
Aug 29, 2026
Docs and dead files: - Remove the GLM5.3-Flash tutorial until the model is validated, together with its mkdocs nav entry and its supported_models row. Shipping a guide and a support claim points users at a path that is not yet verified across the A2/A3/A5 version pairings. - Remove tests/e2e/models/configs/GLM-5.3-Flash.yaml. It is absent from the accuracy.txt config list that drives the eval runs, referenced nowhere else, and its only metric is a 0.0 placeholder, so nothing ever ran it. Code: - Import the PyTorch causal_conv1d from vllm_ascend.ops instead of the 310P-private package. The module is a plain PyTorch reference with no 310P hardware logic and had no consumer inside _310p/, so reaching into that directory only coupled the GLM path to an unrelated hardware target. - Replace the two print calls in patch_triton with logger calls. The NPU Triton causal_conv1d_update was removed upstream in vllm-project#14620, so the failure branch fires on every start; it now names the consequence (the PyTorch fallback syncs per request and therefore stalls ACL graph capture at decode-FULL) instead of writing a bare repr to stdout. - Use isinstance(spec, KpoolTailSpec) instead of comparing type names, which matches the isinstance checks already used in the same function. - Restrict the MTP shared_head shape-match sharing to tied architectures. GLM-5.3-Flash omits shared_head.head from its checkpoint so the weights never compare equal, but any other MTP checkpoint may ship an independently trained head of the same shape, which would have been silently replaced by the target lm_head, costing acceptance rate with nothing in the logs to explain it. - Warn when the MLA rope cache cannot return a slice of the persistent buffer. Callers passing use_cache=True rely on fixed addresses for ACL graph replay, so this path is only safe outside a capture region. Adds unit tests for the shared_head sharing rules, covering the tied architecture, the independently trained head, the preserved value-equality behaviour and a shape mismatch. Signed-off-by: yiminghub2024 <482890@qq.com> Co-authored-by: Cursor <cursoragent@cursor.com>
yiminghub2024
added a commit
to yiminghub2024/vllm-ascend
that referenced
this pull request
Aug 29, 2026
Docs and dead files: - Remove the GLM5.3-Flash tutorial until the model is validated, together with its mkdocs nav entry and its supported_models row. Shipping a guide and a support claim points users at a path that is not yet verified across the A2/A3/A5 version pairings. - Remove tests/e2e/models/configs/GLM-5.3-Flash.yaml. It is absent from the accuracy.txt config list that drives the eval runs, referenced nowhere else, and its only metric is a 0.0 placeholder, so nothing ever ran it. Code: - Import the PyTorch causal_conv1d from vllm_ascend.ops instead of the 310P-private package. The module is a plain PyTorch reference with no 310P hardware logic and had no consumer inside _310p/, so reaching into that directory only coupled the GLM path to an unrelated hardware target. - Replace the two print calls in patch_triton with logger calls. The NPU Triton causal_conv1d_update was removed upstream in vllm-project#14620, so the failure branch fires on every start; it now names the consequence (the PyTorch fallback syncs per request and therefore stalls ACL graph capture at decode-FULL) instead of writing a bare repr to stdout. - Use isinstance(spec, KpoolTailSpec) instead of comparing type names, which matches the isinstance checks already used in the same function. - Restrict the MTP shared_head shape-match sharing to tied architectures. GLM-5.3-Flash omits shared_head.head from its checkpoint so the weights never compare equal, but any other MTP checkpoint may ship an independently trained head of the same shape, which would have been silently replaced by the target lm_head, costing acceptance rate with nothing in the logs to explain it. - Warn when the MLA rope cache cannot return a slice of the persistent buffer. Callers passing use_cache=True rely on fixed addresses for ACL graph replay, so this path is only safe outside a capture region. Adds unit tests for the shared_head sharing rules, covering the tied architecture, the independently trained head, the preserved value-equality behaviour and a shape mismatch. Signed-off-by: yiminghub2024 <482890@qq.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Lethobenthos20
pushed a commit
to Lethobenthos20/vllm-ascend
that referenced
this pull request
Sep 4, 2026
### What this PR does / why we need it? This PR removes the obsolete Ascend Triton causal_conv1d_update_npu implementation. - Delete the Triton kernel and Python wrapper. - Stop monkey-patching the vLLM causal_conv1d_update function, leaving the upstream implementation in place. - Remove the two skipped tests that only exercised the deleted operator. - Keep the shared extract_last_width helper and the existing AscendC and 310P causal-conv paths unchanged. The removed tests documented an overflow case and a probabilistic failure, so they did not provide active coverage for this implementation. ### Does this PR introduce _any_ user-facing change? No public API or configuration change. Internally, vLLM-Ascend no longer replaces the vLLM causal_conv1d_update function with this Triton implementation. ### How was this patch tested? - Full repository checks: bash format.sh ci - Python syntax compilation for the three changed files - Verified there are no remaining references to causal_conv1d_update_npu or its Triton kernel NPU operator tests were not run locally because the operator and its skipped tests are removed by this change. - vLLM version: v0.27.1 - vLLM main: vllm-project/vllm@58d3918 Signed-off-by: maoxx241 <maomaoyu870@gmail.com>
tracellex
pushed a commit
to tracellex/vllm-ascend
that referenced
this pull request
Sep 27, 2026
…oject#14620 vllm-project#14620 (0825) removed the Ascend Triton causal_conv1d_update_npu as "obsolete"; vllm-project#15127 (0903) then re-wired patch_triton.py to import it for the GLM-5.3-Flash KDA layers, leaving every 0.29-line deployment binding the PyTorch fallback instead (patch_triton.py:326 ImportError -> :332 warning -> host-syncing fallback on every decode step). Kernel body is the upstream tiled implementation from f0a9389~1 (588-line tree), kept line-identical. Wrapper is re-bound to the vllm 0.28/0.29 call contract: - stride-rebinding writes conv_state updates in place instead of the upstream transpose().contiguous() copies (which silently dropped state updates); - null_block_id defaults to the NULL_BLOCK_ID sentinel (int, not None) so the non-spec decode path (validate_data=True, no explicit null_block_id, e.g. kda.py:980) passes the assertion, matching the upstream CUDA/Triton wrapper; Parity vs the vllm upstream Triton kernel on NPU (decoder-0, Triton 3.2.0 / torch 2.10 / CANN 9.1.0), all bit-exact on out+state: 2D non-spec dim512/w4 and dim768/w2; spec varlen two-step sliding window (MAL=4, accepted 4/2/1/3 -> 2/4/1/4); spec 3D; non-spec 3D; null-block sentinel; fp32 observed group only ~1e-3 (fp16 midpoint, in-tolerance). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tracellex
pushed a commit
to tracellex/vllm-ascend
that referenced
this pull request
Sep 30, 2026
…oject#14620 vllm-project#14620 (0825) removed the Ascend Triton causal_conv1d_update_npu as "obsolete"; vllm-project#15127 (0903) then re-wired patch_triton.py to import it for the GLM-5.3-Flash KDA layers, leaving every 0.29-line deployment binding the PyTorch fallback instead (patch_triton.py:326 ImportError -> :332 warning -> host-syncing fallback on every decode step). Kernel body is the upstream tiled implementation from f0a9389~1 (588-line tree), kept line-identical. Wrapper is re-bound to the vllm 0.28/0.29 call contract: - stride-rebinding writes conv_state updates in place instead of the upstream transpose().contiguous() copies (which silently dropped state updates); - null_block_id defaults to the NULL_BLOCK_ID sentinel (int, not None) so the non-spec decode path (validate_data=True, no explicit null_block_id, e.g. kda.py:980) passes the assertion, matching the upstream CUDA/Triton wrapper; Parity vs the vllm upstream Triton kernel on NPU (decoder-0, Triton 3.2.0 / torch 2.10 / CANN 9.1.0), all bit-exact on out+state: 2D non-spec dim512/w4 and dim768/w2; spec varlen two-step sliding window (MAL=4, accepted 4/2/1/3 -> 2/4/1/4); spec 3D; non-spec 3D; null-block sentinel; fp32 observed group only ~1e-3 (fp16 midpoint, in-tolerance). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tracellex
pushed a commit
to tracellex/vllm-ascend
that referenced
this pull request
Oct 8, 2026
合并 feat/glm5-kpool-ascendc-indexer 44 commits + 队友线 2 commits: - glm5_kpool_split AscendC 内核转正(split 达成: 64k TTFT -37.5% / 192k -59% vs triton) - glm5_kpool_indexer: dispatcher PR#17542 契约补全 + padded rows mask + absolute positions - 上游 backport: PR#17115/vllm-project#17613/vllm-project#17542/vllm-project#17524(P/D FullAttn TP parallel / mooncake) - Restore causal_conv1d_update_npu Triton kernel (dropped by vllm-project#14620) - memcache: filter cacheable KV groups + rewarm SSD on load - ascend_store: port vllm-project#16945 empty RoPE view regression tests 原始历史: backup/glm5.3-flash-0.29-pre-squash (bd6c295)
tracellex
pushed a commit
to tracellex/vllm-ascend
that referenced
this pull request
Oct 8, 2026
…atcher integration - glm5_kpool_split AscendC kernels replacing the Triton path (64k TTFT -37.5%, 192k -59%) - glm5_kpool_indexer: AscendC implementations (arch22/arch35), stability hardening (PIPE_M event lifecycle, CrossCore MODE2), topk competition fixes, merge-fold and lazy fold - Complete dispatcher PR#17542 contract; mask padded rows; read absolute positions - Restore causal_conv1d_update_npu Triton kernel dropped by vllm-project#14620 - Filter cacheable KV groups and rewarm SSD on memcache load - Port vllm-project#16945 empty RoPE view regression tests
1 task done
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?
This PR removes the obsolete Ascend Triton causal_conv1d_update_npu implementation.
The removed tests documented an overflow case and a probabilistic failure, so they did not provide active coverage for this implementation.
Does this PR introduce any user-facing change?
No public API or configuration change. Internally, vLLM-Ascend no longer replaces the vLLM causal_conv1d_update function with this Triton implementation.
How was this patch tested?
NPU operator tests were not run locally because the operator and its skipped tests are removed by this change.