Skip to content

Conversation

@ZLiao097
Copy link
Contributor

@ZLiao097 ZLiao097 commented Oct 27, 2025

What does this PR do?

Add concise overview of what this PR aims to achieve or accomplish. Reference related GitHub issues and PRs that help with the review.

Checklist Before Starting

  • Search for similar PRs. Paste at least one query link here: [fsdp] feat: add NPU fusion kernels for Qwen3 MoE #3221
  • Format the PR title as [{modules}] {type}: {description} (This will be checked by the CI)
    • {modules} include fsdp, megatron, sglang, vllm, rollout, trainer, ci, training_utils, recipe, hardware, deployment, ray, worker, single_controller, misc, perf, model, algo, env, tool, ckpt, doc, data
    • If this PR involves multiple modules, separate them with , like [megatron, fsdp, doc]
    • {type} is in feat, fix, refactor, chore, test
    • If this PR breaks any API (CLI arguments, config, function signature, etc.), add [BREAKING] to the beginning of the title.
    • Example: [BREAKING][fsdp, megatron] feat: dynamic batching

Test

For changes that can not be tested by CI (e.g., algorithm implementation, new model support), validate by experiment(s) and show results like training curve plots, evaluation results, etc.

Tested with Qwen2-32B on Ascend A2, test results with train_prompt_bsz=512 sp8 fsdp64 on Ascend A2. The red line represents without fused operator, the yellow line represents NPU with fused operator.
Rewards:
reward

throughput:
throughput

API and Usage Example

Demonstrate how the API changes if any, and provide usage example(s) if possible.

# Add code snippet or script demonstrating how to use this

Design & Code Changes

Demonstrate the high-level design if this PR is complex, and list the specific changes.

Checklist Before Submitting

Important

Please check all the following items before requesting a review, otherwise the reviewer might deprioritize this PR for review.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces NPU fusion kernel patches for the Qwen2 model and refactors existing patches to be more generic. The changes are well-structured, particularly the addition of .contiguous() calls in apply_rotary_pos_emb_npu, which is a crucial correctness fix for NPU operations. My review includes a suggestion to improve code clarity by reverting a function rename to be more specific to its usage, which will enhance long-term maintainability.

@ZLiao097 ZLiao097 changed the title [fsdp] feat: add NPU fusion kernels for Qwen3 MoEAdd the Qwen2 model fusion operator. [fsdp] feat: add NPU fusion kernels for Qwen2 and Qwen2.5 dense model. Oct 27, 2025
@ZLiao097
Copy link
Contributor Author

@FightingZhen The issues raised have been resolved. Please review them again. : )

@ZLiao097 ZLiao097 requested a review from FightingZhen October 31, 2025 03:02
@FightingZhen
Copy link
Collaborator

@ZLiao097 please pull and rebase the newest code from main, there are some modifications about e2e_ascend.

@ZLiao097
Copy link
Contributor Author

ZLiao097 commented Nov 3, 2025

@ZLiao097 please pull and rebase the newest code from main, there are some modifications about e2e_ascend.

Done~

@ZLiao097 ZLiao097 requested a review from FightingZhen November 3, 2025 07:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants