Skip to content

Commit 09290a3

Browse files
py-ai-devclaude
andcommitted
Fix fused RoPE backward pass computing wrong gradients (#2)
_FusedRoPE.backward reused the forward kernel verbatim with the same sin, but RoPE's forward is a rotation matrix [[c,-s],[s,c]] applied to each (x0,x1) pair, so the correct backward is that matrix's transpose — equivalent to the same kernel with sin negated, not reapplied unchanged. Every gradient through the fused path was wrong whenever sin != 0 (max abs error 3.64 in the reported repro vs 0.0 after the fix), silently corrupting training for any model using RotaryEmbedding(use_fused=True). test_gradient_flows only checked grad is not None, so this shipped undetected. Added test_gradient_correctness, which compares the fused path's gradient against the vanilla autograd-correct path via assert_close, and fails against the old code (85% of elements mismatched) while passing with the fix. Reported by @tohtana during review of deepspeedai/DeepSpeed#8052. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 4dad48e commit 09290a3

3 files changed

Lines changed: 41 additions & 2 deletions

File tree

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "torchembed"
7-
version = "0.3.0"
7+
version = "0.3.1"
88
description = "A PyTorch library of modern embedding strategies missing from torch.nn"
99
readme = "README.md"
1010
license = { file = "LICENSE" }

tests/test_positional.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,42 @@ def test_gradient_flows(self, dim):
140140
assert q.grad is not None
141141
assert k.grad is not None
142142

143+
def test_gradient_correctness(self, dim):
144+
"""Fused kernel gradients must numerically match the vanilla path's.
145+
146+
RoPE's forward is a per-position rotation matrix [[c, -s], [s, c]] applied
147+
to each (x0, x1) pair; its backward is that matrix's transpose, not the
148+
forward transform reapplied unchanged. A prior bug reused the forward
149+
kernel verbatim for backward, which silently produced wrong gradients
150+
for every dim with sin != 0. `torch.testing.assert_close` on the grads
151+
(not just shape/is-not-None) is what catches a regression here.
152+
"""
153+
if not torch.cuda.is_available():
154+
pytest.skip("CUDA not available")
155+
rope = RotaryEmbedding(dim=dim, device="cuda")
156+
cos, sin = rope.cos_cache[:8], rope.sin_cache[:8]
157+
158+
torch.manual_seed(0)
159+
q_base = torch.randn(2, 4, 8, dim, device="cuda")
160+
k_base = torch.randn(2, 4, 8, dim, device="cuda")
161+
grad_q_up = torch.randn_like(q_base)
162+
grad_k_up = torch.randn_like(k_base)
163+
164+
q_ref = q_base.clone().requires_grad_(True)
165+
k_ref = k_base.clone().requires_grad_(True)
166+
q_rot_ref, k_rot_ref = rope(q_ref, k_ref)
167+
q_rot_ref.backward(grad_q_up, retain_graph=True)
168+
k_rot_ref.backward(grad_k_up)
169+
170+
q_fused = q_base.clone().requires_grad_(True)
171+
k_fused = k_base.clone().requires_grad_(True)
172+
q_rot_fused, k_rot_fused = rope._fused_forward(q_fused, k_fused, cos, sin)
173+
q_rot_fused.backward(grad_q_up, retain_graph=True)
174+
k_rot_fused.backward(grad_k_up)
175+
176+
torch.testing.assert_close(q_fused.grad, q_ref.grad, atol=1e-4, rtol=1e-4)
177+
torch.testing.assert_close(k_fused.grad, k_ref.grad, atol=1e-4, rtol=1e-4)
178+
143179
def test_fused_forward_function(self, dim):
144180
"""Direct call to fused_rope_forward should match."""
145181
if not torch.cuda.is_available():

torchembed/_triton.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,10 @@ def forward(ctx, x, cos, sin):
9191
@staticmethod
9292
def backward(ctx, grad_output):
9393
cos, sin = ctx.saved_tensors
94-
grad_input = _fused_rope_forward_core(grad_output, cos, sin)
94+
# Forward applies rotation matrix [[c, -s], [s, c]] to x. Its gradient
95+
# w.r.t. x is the transpose [[c, s], [-s, c]], which is the same rotation
96+
# kernel run with sin negated (a rotation matrix's transpose is its inverse).
97+
grad_input = _fused_rope_forward_core(grad_output, cos, -sin)
9598
return grad_input, None, None
9699

97100
def _fused_rope_forward_core(x, cos, sin):

0 commit comments

Comments
 (0)