Repository navigation
Conversation
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 introduces support for Decision 3.0 (D3) models on Ascend hardware by implementing a new model adapter. It leverages the existing Ascend Qwen3.5 multimodal backbone and integrates vLLM's sequence-classification capabilities. The changes include custom processing info for D3, weight mapping adjustments, and the ability to load secondary readout weights, ensuring compatibility with index-less checkpoints. 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
|
|
👋 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}
There was a problem hiding this comment.
Code Review
Suggested PR Title:
[Model][Feature] Support D3ForSequenceClassification model on AscendSuggested PR Summary:
### What this PR does / why we need it?
This pull request registers and implements the `D3ForSequenceClassification` model (based on `AscendQwen3_5ForConditionalGeneration`) for the Ascend backend. It adds processing info (`D3ProcessingInfo`) to handle image and video token sizes and configures secondary weights for the readout.
However, there is a critical issue in `vllm_ascend/models/d3.py` where `allow_patterns_overrides` is used instead of `allowed_patterns` in `DefaultModelLoader.Source`, which will cause a `TypeError` at runtime.
### Does this PR introduce _any_ user-facing change?
Yes, it adds support for the `D3ForSequenceClassification` model.
### How was this patch tested?
New tests have been added:
- `tests/e2e/pull_request/one_card/pooling/test_d3.py` for end-to-end pooling and endpoint tests.
- `tests/ut/models/test_d3_registration.py` for unit testing model registration and unused weight checks.
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
do420
changed the title
The pooling branch of AscendAttentionBackendImpl.forward chose the bidirectional path from attn_metadata.causal alone. ENCODER_ONLY layers of hybrid models (Gated DeltaNet + full attention, e.g. Qwen3.5-based D3) reach the backend with causal=True because they do not go through the EncoderOnlyAttention wrapper, so they silently ran causal attention. Also route layers whose attn_type is ENCODER_ONLY to _forward_encoder_attention, as upstream backends do. DECODER layers are unchanged. Add unit tests; the ENCODER_ONLY one fails without the change. Signed-off-by: do420 <denizozcanmc@gmail.com>
545b0e4 to
6cbafff
Compare
The encoder attention path attends over rank-local K/V only; unlike the KV-cache write it never gathers the other PCP ranks' tokens, so bidirectional attention would silently be wrong. Pooling models default to the V1 model runner, which rejects PCP, but VLLM_USE_V2_MODEL_RUNNER=1 can force V2 and bypass that check. Fail at model construction instead, add unit tests for the guard and for the V1 rejection, and document the limitation. Signed-off-by: do420 <denizozcanmc@gmail.com>
Adapt the upstream D3 scoring model to the Ascend Qwen3.5 path and add model registration and System One regression coverage. Signed-off-by: do420 <denizozcanmc@gmail.com>
Skip the readout tensor during the backbone weight pass so index-less D3 checkpoints such as d3-nano still load their decision head through secondary weights. Cover d3-edge and d3-nano in the checkpoint differential test. Signed-off-by: do420 <denizozcanmc@gmail.com>
Exercise the index-less D3 nano checkpoint in the pooling differential and System One endpoint tests. Signed-off-by: do420 <denizozcanmc@gmail.com>
Match upstream differential coverage for both the edge baseline and nano sidecar-loading case; retain the endpoint smoke test on edge. Signed-off-by: do420 <denizozcanmc@gmail.com>
Record the user-visible Ascend D3 adapter and its dependency on upstream native D3 and System One support. Signed-off-by: do420 <denizozcanmc@gmail.com>
Disable media inputs in the D3 pooling and System One tests: graph mode with multimodal inputs enabled currently fails during startup profiling for D3's ENCODER_ONLY attention layers, and the tests only send text. Raise the softmax tolerance from 0.02 to 0.03: d3-nano reaches about 0.019 against the checkpoint runtime, which left almost no headroom, while the causal-attention regression this test guards against shows differences above 0.2. Signed-off-by: do420 <denizozcanmc@gmail.com>
The graph-mode startup failure seen with media inputs enabled came from a stale torch.compile cache written by a run with a different limit_mm_per_prompt setting (the cache key does not include it), not from D3's ENCODER_ONLY layers. With a clean cache, graph mode with media enabled works. Keep media disabled since the tests only send text, and document the cache-key caveat. Signed-off-by: do420 <denizozcanmc@gmail.com>
QUESTIONS mixed str, dict and list values, so mypy inferred object and rejected indexing it in the System One test on every supported Python version. Signed-off-by: do420 <denizozcanmc@gmail.com>
The test coverage config check requires every e2e test file under tests/e2e/pull_request to have an estimated time; test_d3.py was missing, which fails the select-tests job. Signed-off-by: do420 <denizozcanmc@gmail.com>
The release tag matrix (main2main label) runs the e2e suite on v0.31.0, which has no D3 model. Use the existing vllm_version_is gate so the test is skipped there. Signed-off-by: do420 <denizozcanmc@gmail.com>
6cbafff to
d185e6f
Compare
…rsion Native D3 (vLLM #61565, #61778) is in neither v0.31.0 nor v0.32.0, so the exact-version skip let the tests run and fail on v0.32.0. Probe for vllm.model_executor.models.d3 instead. Signed-off-by: do420 <denizozcanmc@gmail.com>
d185e6f to
2ead1cb
Compare
Signed-off-by: Deniz Özcan <66875687+do420@users.noreply.github.com>
Bring in the verified vLLM 24420b7a pin alongside the Qwen4 indexer imports. The previous CI run combined current Ascend sources with the older 544f182d pin and failed while importing the unit-test conftest. Validation: bash format.sh ci; verified that the new vLLM pin defines QSAIndexerMetadataBuilder in qsa_indexer.py. Runtime CPU and NPU tests require CI hardware and dependencies. Signed-off-by: do420 <denizozcanmc@gmail.com>
Stack the changes from vllm-project#19919 so the D3 numerical tests exercise the required ENCODER_ONLY attention behavior. Signed-off-by: do420 <denizozcanmc@gmail.com>
Install accelerate for the checkpoint runtime in E2E CI. Bind the D3 server to the same loopback host and port used by its health check and System One request. Signed-off-by: do420 <denizozcanmc@gmail.com>
github-actions
Bot
added
the
documentation
|
/rerun Rerun (failed jobs only):
|
Keep pooling parity and System One coverage on the smaller checkpoint. Allow the test environment to resolve accelerate without a version floor. Signed-off-by: do420 <denizozcanmc@gmail.com>
|
/rerun Rerun (failed jobs only):
|
1 similar comment
|
/rerun Rerun (failed jobs only):
|
|
Thanks for your contribution! Can you explain why we can't inherit vLLM model strucutre in the PR description? |
What this PR does / why we need it?
Adds an Ascend adapter and model registration for upstream Decision 3.0 (D3) checkpoints. D3 uses a Qwen3.5 multimodal backbone with a separate answer-code readout. The adapter reuses Ascend's Qwen3.5 path and vLLM's sequence-classification/pooling interface, and loads
readout.safetensorsthrough the model's secondary-weight source. It also skips the readout tensor in the backbone weight pass so index-less checkpoints such as d3-nano can load it through the sidecar path.The PR does not implement a parallel decision endpoint. It is intended to use upstream vLLM's native
POST /v1/systemoneroute and D3 model implementation. The upstream implementation landed in vLLM #61565, with additional checkpoint-size support in #61778.Stacked on #19919. This branch includes the bidirectional attention fix required for D3’s
ENCODER_ONLYlayers and their numerical comparisons (see #19918).This branch is synced with vLLM-Ascend
mainat219f44caand inherits its verified vLLM pin,24420b7a, without a D3-specific pin override.Does this PR introduce any user-facing change?
Yes. It adds an Ascend model adapter for D3 sequence-classification checkpoints, enabling D3 decisions through upstream System One.
How was this patch tested?
Passed
D3 E2E setup (
b41f8c8a): addedaccelerateto the test dependencies and used one loopback port for the System One server, health check, and request.bash format.sh cipassed, and a local HTTP harness verified that the health check and request use the launched port. The model and NPU assertions await CI.CI compatibility sync (
3e7f6367): merged upstreammainto align the Ascend Qwen4 indexer imports with the verified vLLM24420b7apin. The preceding CPU job used544f182d, which lacksqsa_indexer.py, and failed during conftest import before any tests ran. Confirmed the new pin containsQSAIndexerMetadataBuilder;bash format.sh cipasses locally on the synced tree. Both CPU jobs passed in the CI run.CPU-only validation of current head
b41f8c8aagainst vLLM24420b7a:python -m pytest -q tests/ut/models/test_d3_registration.py tests/ut/attention/test_attention_v1.py tests/ut/test_platform.py— 208 passed, 1 skipped. The container had no NPU devices, 2 CPUs and a 12 GiB memory limit.Checked the pinned vLLM
24420b7aloader API on CPU:DefaultModelLoader.Sourceacceptsallow_patterns_overrides; the bot-suggestedallowed_patternsraisesTypeError. The existing readout source argument is correct.bash format.sh ci: all hooks pass.TORCH_DEVICE_BACKEND_AUTOLOAD=0 pytest -q tests/ut/models/test_d3_registration.py: 2 passed on the rebased branch, against vLLM544f182d(CPU only).The e2e tests are skipped unless
vllm.model_executor.models.d3is importable (native D3 is in neither v0.31.0 nor v0.32.0).Ruff check and format check on the D3 implementation and test files: passed.
Ascend 950DT, CANN 9.2.0.b080, torch_npu 2.10.0.post4, vLLM
c0b6f74(the pin onmainwhen this comparison was run), this branch plus the fix from [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919:d3-edgeandd3-nanoloaded through the sidecar path and classified withrunner="pooling",max_model_len=4096, media inputs disabled.Pooling output compared with the checkpoint runtime (
AutoModel.from_pretrained(..., trust_remote_code=True)), softmax over each question's options, three questions (choice, noul, score):Without the fix from [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919 the same comparison gives 0.33 on d3-edge.
POST /v1/systemoneon d3-edge (Choice, Noul and Score in one request) returned HTTP 200 with probabilities matching the offline run.Re-ran the same comparison after rebasing onto a newer vLLM-Ascend
main(one commit later than the544f182dpin above), against vLLM766c837b(the merge commit of upstream #61778, so it carries both #61565 and #61778): same hardware, same fix from [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919. Softmax differences matched exactly (d3-edge eager and graph 0.0065, d3-nano graph 0.0193), so the result is not tied to one specific vLLM/vLLM-Ascend commit pair.System One Bench accuracy (benchmark results).
Accuracy is the share of correct decisions across 2,200 labeled examples per model. The benchmark contains 2,000 Typed Decisions examples and 200 AG News topic-classification examples. The CPU baseline totals are calculated from the published subset scores, weighted by their example counts. Both Ascend runs completed 600 requests with zero failures.
Jev Decision Index, BANKING77 (evaluation kit, leaderboard results). Each model completed all 3,080 requests, with zero failures. Macro-F1 is the leaderboard metric.
The published macro-F1 scores are 89.97% for d3-edge and 91.33% for d3-nano. The Ascend results are 0.12 and 0.06 percentage points lower, respectively.
Both benchmarks used one Ascend 950DT per model in eager mode via
POST /v1/systemone, with media disabled andmax_model_len=4096. The scratch stack used the D3 adapter, the [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919 attention fix, and vLLM544f182d. The Jev results cover BANKING77 only. The current24420b7apin is outside the scope of both runs.A first, lightweight latency pass. Measurement environment caveat: this ran in a scratch setup, not the configuration used for the accuracy numbers above: a generic
vllm-ascend:nightly-main-a5-openeulerimage with this branch and a standalone vLLM source tree (766c837b) overlaid on top of the image's own installed packages viaPYTHONPATH, rather than a normalpip install, one card,gpu_memory_utilizationdeliberately low (0.2-0.25) to assume a shared card, one engine per run, 3-question batch, 20 repeats after a warmup call. Treat the numbers below as a rough first signal, not a benchmark from a representative deployment:Note: graph mode was slightly slower than eager for d3-edge here. The batch is tiny (3 requests) and the model is small (12 layers), so graph capture overhead may outweigh its benefit at this scale; this is not a claim that graph mode regresses D3 in general, and it should be re-measured under a larger batch/concurrent load before drawing that conclusion.
System One serving performance and replica scaling. Measured successful requests per second and end-to-end p99 HTTP latency through
POST /v1/systemone. Each request contained a synthetic 330-token support ticket and one choice question (436 input tokens after formatting). Closed-loop clients kept one request in flight each. Each point was measured once for 15 seconds after a 3-second warm-up.The scratch stack used vLLM
544f182d, the D3 adapter, and the [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919 attention fix. Each replica ran in eager mode on its own Ascend 950DT, with prefix caching and media disabled,max_model_len=4096, andmax_num_seqs=64. In the two-replica run, clients were evenly split between the two endpoints. The one-replica rows were remeasured in the same sweep.How to read the tables. Throughput counts successful responses per second across all cards. The p99 response time is the time within which about 99% of successful requests completed. For example, at 32 concurrent clients, d3-edge on two cards served 779.2 requests per second and about 99% of successful requests finished within 56 ms.
Initial single-card sweep
This separate run measured each model on one card at 1–64 concurrent clients. The tables below it use the later replica-scaling run, which remeasured the overlapping one-card settings.
The Edge client recorded two read errors at 64 clients. All other measured requests in this sweep succeeded.
Replica scaling sweep
d3-edge
d3-nano
Peak measured throughput increased by 1.72× for d3-edge and 2.00× for d3-nano. The client logged one read error each for d3-edge at 64 and 256 clients on one card and at 256 clients on two cards. For d3-nano it logged three at 256 clients on one card, one at 128 clients on two cards, and three at 256 clients on two cards. Other points had zero request errors.
Validation limitations
tests/e2e/pull_request/one_card/pooling/test_d3.pyas pytest on Ascend, unmodified. Both numeric-comparison parametrizations passed:test_d3_pooling_matches_checkpoint_runtime[vllm-sr/d3-edge]and[vllm-sr/d3-nano](1 passedeach). The environment has no outbound access to huggingface.co, so the two model directories were made available under the working directory asvllm-sr/d3-edgeandvllm-sr/d3-nano(both resolve locally before any Hub lookup is attempted, in transformers, vLLM and the D3 runtime's ownresolve_dir); no test code was changed. d3-edge's checkpoint ships without aMODEL_MANIFEST.json-matchingmodel.safetensors.index.json, so one was generated from the safetensors header and the checkpoint'sMODEL_MANIFEST.jsonwas left out of the local copy, which skips the (otherwise inapplicable) manifest hash check.test_d3_systemone_endpointdid not reach a verdict in this run: the server started and answered/healthnormally, but the request from the test itself did not return within 10 minutes and the run was stopped. This looks environment-specific (unrelated runs on this same card show an occasional long one-time delay around the first full decode-capable graph build, logged as a Tritoncausal_conv1d_updatefallback that "stalls ACL graph capture at decode-FULL"); the System One endpoint itself was separately confirmed working over HTTP, with matching probabilities, in the manual run above.torch.compilecache can crash startup withValueError: not enough values to unpack (expected 6, got 5)when it was written by a run with a differentlimit_mm_per_promptsetting (for example media limited to zero, then enabled, or the reverse). The cache key (MultiModalConfig.compute_hash) does not includelimit_per_prompt, so both settings share one cache directory. With a clean cache, d3-edge in graph mode with media enabled works (max softmax difference 0.0065), and re-running the same configuration from its own cache works. This is a known upstream vLLM issue (vllm-project/vllm#50891, fix proposed in vllm-project/vllm#50908) and is not caused by the D3 adapter or by the attention fix in [Attention][BugFix] Ensure ENCODER_ONLY layers in pooling models bypass causal path #19919. The e2e tests keep media inputs disabled because they only send text.Checklist
bash format.sh ciwith pre-commit available (all hooks pass, gitleaks and shellcheck installed locally).