Skip to content

Fix: Correct cross-attention query routing in Post-LN TransformerDecoderLayer - #3382

Merged
angeloskath merged 2 commits into
ml-explore:mainfrom
suryawanshishantanu6:fix/transformer-post-ln-routing
Apr 7, 2026
Merged

angeloskath merged 2 commits into
ml-explore:mainfrom
suryawanshishantanu6:fix/transformer-post-ln-routing

Conversation

@suryawanshishantanu6

Copy link
Copy Markdown
Contributor

Proposed changes

This PR fixes a critical data flow bug in TransformerDecoderLayer when initialized with norm_first=False.

Previously, the cross-attention module was receiving y (the raw, dropped-out output of the self-attention layer) as its query. This inadvertently bypassed the first LayerNorm (self.ln1) and severed the residual connection (x + y). I updated the query input to x to correctly route the output of the first Add & Norm block into the cross-attention block, aligning with standard Post-LN Transformer architecture.

Checklist

  • [ x ] I have read the CONTRIBUTING document
  • [ x ] I have run pre-commit run --all-files to format my code / installed pre-commit prior to committing changes
  • [ x ] I have added tests that prove my fix is effective or that my feature works

@angeloskath angeloskath left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for catching that!

I removed the last test because it is basically a re-implementation of the layer so it doesn't really help anything. If there is a bug then it is in both and if it is in another component then it will be present in both.

@suryawanshishantanu6

Copy link
Copy Markdown
Contributor Author

Thanks for catching that!

I removed the last test because it is basically a re-implementation of the layer so it doesn't really help anything. If there is a bug then it is in both and if it is in another component then it will be present in both.

Perfect, thanks for the review and taking care of the test!

@angeloskath
angeloskath merged commit a8776b7 into ml-explore:main Apr 7, 2026
16 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Apr 22, 2026
1 task done
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