Add DeepSeekV4 NPU support#27
Conversation
![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 introduces support for DeepSeek-V4 on Ascend NPU, providing environment setup documentation, installation scripts, and patches for mbridge, Megatron-LM, verl, and vllm-ascend. The review identified several critical issues: hardcoded absolute paths for CANN versions in the verl patch reduce portability, and generic exception handling during weight loading in vllm-ascend could lead to silent failures. Furthermore, an unconditional configuration change in Megatron-LM disables GQA/MQA globally, and emptying shared configuration keys in verl may break other training recipes.
| + os.environ[ | ||
| + "ASCEND_CUSTOM_OPP_PATH"] = f"/usr/local/Ascend/cann-9.0.0-beta.2/opp/vendors/custom_transformer" |
There was a problem hiding this comment.
The ASCEND_CUSTOM_OPP_PATH is hardcoded to a specific beta version path (/usr/local/Ascend/cann-9.0.0-beta.2/...). This is inconsistent with the README and install.sh which mention CANN 8.5.0. Hardcoding absolute paths with specific versions makes the code non-portable and prone to breakage across different environments. It is recommended to use an environment variable or derive this path from CANN_INSTALL_PATH. This issue is repeated on lines 230 and 239.
| + except Exception as e: | ||
| + print(f"{name} 发生错误: {e}") |
There was a problem hiding this comment.
Catching a generic Exception and only printing the error during weight loading is risky. If a weight fails to load, the model will continue running with uninitialized or incorrect parameters, leading to silent failures or incorrect outputs. These errors should be handled more robustly, or at least re-raised after logging to ensure the process stops if the model state is invalid. This pattern is repeated on lines 41, 95, and 125.
| + #if self.num_query_groups is None: | ||
| + self.num_query_groups = self.num_attention_heads |
There was a problem hiding this comment.
The assignment self.num_query_groups = self.num_attention_heads is now unconditional, which effectively disables Grouped-Query Attention (GQA) and Multi-Query Attention (MQA) for all models using this configuration. This change should be guarded or moved to a model-specific configuration to avoid breaking other architectures that rely on num_query_groups being different from num_attention_heads.
| - initial_loss_scale: 4096 | ||
| - init_method_std: 0.01 | ||
| - hidden_dropout: 0.0 | ||
| +llm_kwargs: {} |
There was a problem hiding this comment.
Emptying llm_kwargs in the shared mindspeed.yaml configuration file removes many important default settings (e.g., use_mcore_models, qk_layernorm, swiglu). This change will likely break other training recipes or models that depend on these defaults. It is better to provide a specific configuration file for DeepSeek-V4 or only override the necessary keys.
Co-authored-by: psyloy <15966303071@163.com> Co-authored-by: yyyy2000 <yezhibei@huawei.com> Co-authored-by: autbuster <190498533@qq.com>
| cd Megatron-LM | ||
| git checkout core_v0.12.1 | ||
| cd .. | ||
| git clone https://gitcode.com/ascend/MindSpeed-LLM.git |
2 participants
What does this PR do?
Add DeepSeekV4 support on NPU
Test
environment and hyperparameters
Hardware: 8 x Atlas A3
Model: DeepSeekV4-Flash-Base
Algorithm:GRPO
max_response_length: 2048
key metrics
training/rollout_probs_diff_mean: 0.0068
training/rollout_probs_diff_std:0.0160
training/rollout_actor_probs_pearson_corr:0.9991
critic/rewards/mean(step 100): 0.9316
eval
API and Usage Example
Follow README.md
cd verl bash ../verl-ascend-recipe/deepseekv4/scripts/ray_start.shDesign & Code Changes
Co-authored-by: psyloy 15966303071@163.com
Co-authored-by: yyyy2000 yezhibei@huawei.com
Co-authored-by: autbuster 190498533@qq.com