Compositions

CodeReview

Code review layout with diff viewer, review chat thread, and approval actions.

Preview:
1280px
62
@@ Changes @@
11 class MultiHeadAttention(nn.Module):
22 def __init__(self, d_model, n_heads):
3− self.qkv = nn.Linear(d_model, 3 * d_model)
3+ self.q_proj = nn.Linear(d_model, d_model)
4+ self.k_proj = nn.Linear(d_model, d_model)
5+ self.v_proj = nn.Linear(d_model, d_model)
46 self.out_proj = nn.Linear(d_model, d_model)
@@ Changes @@
1012 def forward(self, x, mask=None):
11− q, k, v = self.qkv(x).chunk(3, dim=-1)
13+ q = self.q_proj(x)
14+ k = self.k_proj(x)
15+ v = self.v_proj(x)
1216 scores = torch.matmul(q, k.transpose(-2, -1))

Review Thread

Why split qkv into separate projections? The fused version is more memory-efficient.
Good point. The separate projections add flexibility for per-head dimension scaling and for swapping positional encodings independently on Q and K. That said, if memory is a priority at this scale we can keep a fused projection and split after the linear. Want me to revise the diff?

Submit Review

Review decision

Choose your review outcome for this pull request.

Medium risk