[BugFix] fix tsp o_proj bias add#5284
Conversation
|
Thanks for your contribution! |
There was a problem hiding this comment.
Pull request overview
This PR attempts to fix an issue related to bias handling in tensor sequence parallelism (TSP) for the o_proj layer. However, the implementation contains a critical bug that needs to be addressed.
Key Issues:
- The PR title lacks the required tag format (should be
[BugFix]or similar) - The PR description is incomplete - it doesn't explain what problem is being fixed or why these modifications are needed
- The code change introduces a bug where variable
kis assigned twice consecutively, causing the first assignment to be ineffective
Comments suppressed due to low confidence (1)
fastdeploy/model_executor/load_weight_utils.py:291
- This assignment to 'k' is unnecessary as it is redefined before this value is used.
k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.weight"
| k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.weight" | ||
| k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.bias" | ||
| if k in weight_list: | ||
| no_tp_action_keys.append(k) |
There was a problem hiding this comment.
The variable k is assigned twice consecutively. The first assignment on line 291 (self_attn.o_proj.weight) is immediately overwritten by line 292 (self_attn.o_proj.weight.bias), making line 291 ineffective.
If both weight and bias keys need to be added to no_tp_action_keys, consider handling them separately:
k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.weight"
if k in weight_list:
no_tp_action_keys.append(k)
k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.bias"
if k in weight_list:
no_tp_action_keys.append(k)Note: The key path also appears incorrect - it should likely be o_proj.bias not o_proj.weight.bias.
| k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.weight" | |
| k = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.bias" | |
| if k in weight_list: | |
| no_tp_action_keys.append(k) | |
| k_weight = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.weight" | |
| if k_weight in weight_list: | |
| no_tp_action_keys.append(k_weight) | |
| k_bias = f"ernie.{prefix_layer_name}.{i}.self_attn.o_proj.bias" | |
| if k_bias in weight_list: | |
| no_tp_action_keys.append(k_bias) |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #5284 +/- ##
==========================================
Coverage ? 60.45%
==========================================
Files ? 322
Lines ? 39176
Branches ? 5885
==========================================
Hits ? 23683
Misses ? 13627
Partials ? 1866
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Motivation
V0 loader下,存在bias的模型,在TSP下不能被除以 tp_size
Modifications
Usage or Command
Accuracy Tests
Checklist
[FDConfig],[APIServer],[Engine],[Scheduler],[PD Disaggregation],[Executor],[Graph Optimization],[Speculative Decoding],[RL],[Models],[Quantization],[Loader],[OP],[KVCache],[DataProcessor],[BugFix],[Docs],[CI],[Optimization],[Feature],[Benchmark],[Others],[XPU],[HPU],[GCU],[DCU],[Iluvatar],[Metax]]pre-commitbefore commit.releasebranch, make sure the PR has been submitted to thedevelopbranch, then cherry-pick it to thereleasebranch with the[Cherry-Pick]PR tag.