Skip to content

Conversation

@Vinkle-hzt
Copy link
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings November 14, 2025 04:00
@Vinkle-hzt Vinkle-hzt requested a review from LLLLKKKK as a code owner November 14, 2025 04:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR attempts to remove the final normalization layer from Python model implementations, but this change introduces a critical bug that will break model inference.

Key Issue

The PR removes self.norm(hidden_states) from the forward methods of both Qwen3Model and GenericMoeModel. However, the C++ wrapper (PyWrappedModel::forward) passes skip_final_layernorm=true to the post-processing layer, which means it expects the Python model to apply the final normalization. Without this normalization:

  1. Hidden states will not be properly normalized before computing logits
  2. Model outputs will be incorrect
  3. Inference results will be degraded or broken

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
rtp_llm/models_py/model_desc/qwen3.py Bug: Removes required final normalization from Qwen3Model.forward()
rtp_llm/models_py/model_desc/generic_moe.py Bug: Removes required final normalization from GenericMoeModel.forward()

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@Vinkle-hzt Vinkle-hzt closed this Nov 17, 2025
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.

1 participant