MTP support DeepSeekV3.2 #4465
MTP support DeepSeekV3.2 #4465
Conversation
|
👋 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
This pull request adds support for the DeepSeekV3.2 model in MTP. The changes are mostly about plumbing new metadata and enabling the model in the configuration. I've found a minor style issue and an opportunity to complete a refactoring for better code consistency. Overall, the changes look good and are in the right direction.
vllm_ascend/attention/sfa_v1.py
Outdated
| query_start_loc = common_attn_metadata.query_start_loc | ||
| query_lens = query_start_loc[1:] - query_start_loc[:-1] | ||
| has_prefill = any(query_lens > self.decode_threshold) | ||
| _, _, num_decode_tokens, _ = split_decodes_and_prefills(common_attn_metadata, decode_threshold = self.decode_threshold) |
There was a problem hiding this comment.
According to PEP 8, you should avoid spaces around the = sign when used for keyword arguments.1
| _, _, num_decode_tokens, _ = split_decodes_and_prefills(common_attn_metadata, decode_threshold = self.decode_threshold) | |
| _, _, num_decode_tokens, _ = split_decodes_and_prefills(common_attn_metadata, decode_threshold=self.decode_threshold) |
Style Guide References
Footnotes
-
PEP 8 suggests not to use spaces around the
=sign when used to indicate a keyword argument. ↩
| decode_meta = getattr(attn_metadata_i, "decode", None) | ||
| prefill_meta = getattr(attn_metadata_i, "prefill", None) |
There was a problem hiding this comment.
You've introduced decode_meta and prefill_meta to cache attribute access, which is a good improvement. However, the following code blocks still use attn_metadata_i.decode and attn_metadata_i.prefill directly. For consistency and improved readability, you should replace those direct accesses with the new local variables throughout this method.
For example, attn_metadata_i.decode.actual_seq_lengths_q on line 802 should be decode_meta.actual_seq_lengths_q, and so on for other occurrences in this method.
ZYang6263
force-pushed
the
pr_ds32_mtp
branch
from
b8b22b9 to
8b251a6
Compare
Signed-off-by: ZYang6263 <[email protected]>
ZYang6263
force-pushed
the
pr_ds32_mtp
branch
from
f372865 to
4d3c88f
Compare
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 participant
What this PR does / why we need it?
Currently, MTP does not support the DeepSeekV3.2 model. In this PR, we have enabled this feature.
Does this PR introduce any user-facing change?
How was this patch tested?