huggingface / huggingface/candle
candle-nn: rotary_emb::rope is apply_op3_no_bwd; severs autograd (same pattern as #2168 / PR #3526)
- Dominant language
- Rust
- Stars
- 21k
- Forks
- 1.8k
- Avg merge
- 16h 42m
- Merged PRs (30d)
- 25
Description
## Severity: HIGH — silently severs autograd in any training path
\`candle_nn::rotary_emb::rope\` at \`candle-nn/src/rotary_emb.rs:537\` uses \`apply_op3_no_bwd\`, so any backward path through it is severed. The caller gets \`None\` from \`grads.get(var)\` after \`loss.backward()\` with no error or warning.
Same pattern present at:
- \`candle-nn/src/rotary_emb.rs:266\` (\`rope_i\`)
- \`candle-nn/src/rotary_emb.rs:808\` (\`rope_thd\`)
The \`_slow\` variants (\`rope_slow\`, \`rope_i_slow\`) are pure-Candle differentiable ops and serve as the workaround. The existence of two variants suggests the no-bwd shape is intentional API design, but the naming doesn't telegraph \"use this one if you need training\" — beginners reach for \`rope\` (the obvious name) and silently lose gradients through attention.
**Verified present at candle main HEAD \`1d7e9274591ef7eb101d965b71243a41994214d2\`** (committed 2026-05-26).
## Reproducer
https://github.com/nxrobins/candle-bug-2-rope-no-bwd
\`\`\`bash
git clone https://github.com/nxrobins/candle-bug-2-rope-no-bwd && cd candle-bug-2-rope-no-bwd && cargo run --release
# exit 1: BUG PRESENT (rope severs autograd; rope_slow propagates)
# exit 0: bug appears fixed
\`\`\`
\`cargo run\` and \`cargo test\` (2 tests) pass; no CUDA/Metal required.
## Suggested fix
Two options:
**Option A (doc-only floor)** — add \`#[doc]\` warnings on \`rope\`/\`rope_i\`/\`rope_thd\` pointing users to the \`_slow\` variants for training, e.g.:
\`\`\`rust
/// NOTE: This is a fused inference-optimized kernel with NO backward pass.
/// Use [\`rope_slow\`] if you need gradients (training, gradient-based gate
/// selection, etc.).
pub fn rope(...) { ... }
\`\`\`
**Option B (supremum)** — implement \`bwd\` for the \`RotaryEmb\` / \`RotaryEmbI\` / \`RotaryEmbThd\` Op structs. The math is the inverse rotation applied to the incoming gradient (RoPE is a rotation matrix; its transpose is its inverse). This would mirror the shape of #3526 (which implements \`bwd\` for the \`RmsNorm\` CustomOp2 and is awaiting review) — the same fast-path-with-backward pattern.
## Related
- #2168 / #3526 — same \`apply_op*_no_bwd\` pattern in \`RmsNorm\` (PR #3526 awaiting review). The fix for rope would mirror that approach.
- #3011 — broader LayerNorm gradient-flow issue.
## Context
Surfaced alongside three other \`_no_bwd\` bugs during Phase 0.5b validation: https://github.com/nxrobins/wave-agent/blob/main/docs/upstream-candle-bugs.md
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at candle-nn/src/rotary_emb.rs lines 266, 537, and 808, then compare the rope, rope_i, and rope_thd implementations with their _slow variants. Run the linked reproducer and existing cargo tests to confirm the missing gradients; done means the chosen fix either documents the training limitation or makes the fast paths propagate gradients, with tests demonstrating the result.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100