huggingface / huggingface/candle

candle-nn: rotary_emb::rope is apply_op3_no_bwd; severs autograd (same pattern as #2168 / PR #3526)

Open
#3,568 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.