Skip to content

fix(rope): don't mark the unused batch-size param as tl.constexpr - #1351

Draft
vaibhavjindal wants to merge 2 commits into
mainfrom
fix/rope-dead-constexpr-batch-size-dynamic-shapes
Draft

fix(rope): don't mark the unused batch-size param as tl.constexpr#1351
vaibhavjindal wants to merge 2 commits into
mainfrom
fix/rope-dead-constexpr-batch-size-dynamic-shapes

Conversation

@vaibhavjindal

Copy link
Copy Markdown
Collaborator

Summary

_triton_rope declares bs: tl.constexpr, but bs is never referenced in the kernel body — it appears only in comments. An AST check of the kernel's parameters:

param      constexpr  used-in-body
  sl         -          yes
  bs         yes        NO   <-- dead
  cos_bs     yes        yes
  n_qh       yes        yes
  ...

Marking a dead parameter constexpr still forces Triton/Dynamo to specialize on it. That breaks torch.compile as soon as the batch dimension becomes dynamic: the value arrives as a SymInt, and Dynamo trips an internal assertion while tracing LigerRopeFunction through the autograd_function_apply HOP:

File "torch/_dynamo/variables/higher_order_ops.py", line 325, in overwrite_tensor_vt_proxy
    assert subgraph_vt.is_tensor() or isinstance(subgraph_vt, SymNodeVariable)
AssertionError:

from user code:
  File "transformers/models/qwen3/modeling_qwen3.py", line 268, in forward
    query_states, key_states = apply_rotary_pos_emb(query_states, key_states, cos, sin)
  File "liger_kernel/ops/rope.py", line 24, in liger_rotary_pos_emb
    return LigerRopeFunction.apply(q, k, cos, sin, position_ids, unsqueeze_dim)

The failure only surfaces on the second distinct input shape — the first call compiles statically, the second triggers automatic dynamic shapes. That timing made it look like an upstream PyTorch bug rather than a kernel signature issue.

Dropping the tl.constexpr annotation is sufficient. The parameter itself is kept so the call signature and the q size: (bsz, ...) documentation stay intact.

Details

Minimal repro (no transformers dependency), against main:

import torch, torch._dynamo as dyn
from liger_kernel.ops.rope import LigerRopeFunction

cf = torch.compile(lambda *a: LigerRopeFunction.apply(*a))   # default: auto-dynamic
def mk(bs, sl, nqh=16, nkv=8, hd=128):
    return (torch.randn(bs, nqh, sl, hd, device="cuda", dtype=torch.bfloat16, requires_grad=True),
            torch.randn(bs, nkv, sl, hd, device="cuda", dtype=torch.bfloat16, requires_grad=True),
            torch.randn(1, sl, hd, device="cuda", dtype=torch.bfloat16),
            torch.randn(1, sl, hd, device="cuda", dtype=torch.bfloat16))

cf(*mk(1, 64))   # OK
cf(*mk(2, 64))   # AssertionError on main

Varying seq_len alone was always fine, because sl is an ordinary runtime arg. Only the constexpr batch size was fatal:

scenario before after
vary seq_len only (auto-dynamic) OK OK
vary batch only (auto-dynamic) FAIL OK
vary batch, dynamic=False OK OK
vary seq_len, dynamic=False OK OK
dynamic=True FAIL FAIL (see below)

End to end, Qwen3-0.6B with apply_liger_kernel_to_qwen3(rope=True, ...) now compiles under a plain torch.compile(model) across all of 1x512, 1x2048, 4x512, 4x2048, 8x1024, 8x2048. Previously that required a dynamic=False workaround. Explicit torch._dynamo.mark_dynamic on the batch and sequence dims also works now (verified at 1x64, 2x128, 8x256).

dynamic=True remains unsupported, and I don't think it should block this. That flag additionally marks head_dim and the head counts dynamic, which cannot work while block sizing is derived from triton.next_power_of_2(head_dim) at launch time. Those dims are fixed by model architecture in practice, so it isn't the case that matters — whereas batch and sequence length genuinely vary, and both work now.

Two related observations, deliberately not changed here:

  1. BLOCK_SIZE is also unused in _triton_rope's body. It's harmless today (it's derived from head counts, so it never becomes a SymInt), so I left it rather than widen the diff.
  2. qwen2vl_mrope.py has the same bs: tl.constexpr annotation, but there bs is genuinely used for pointer arithmetic, so it needs its own treatment rather than the same one-line change. The Ascend/NPU backends carry the same pattern and I have no way to test them here.

Testing Done

  • pytest test/transformers/test_rope.py -> 36 passed

  • pytest test/transformers/test_rope.py test/transformers/test_monkey_patch.py -> 98 passed

  • pytest test/convergence/bf16/test_mini_models.py -k "qwen3 or llama or mistral" -> 11 passed

  • make checkstyle -> passed

  • Compiled vs eager output and gradients are bit-identical (max abs diff 0.0) across bs=1,2,4,8 — expected, since the kernel never reads this parameter.

  • Hardware Type: H100 80GB HBM3

  • run make test to ensure correctness

  • run make checkstyle to ensure code style

  • run make test-convergence to ensure convergence

Note: I ran the targeted suites listed above rather than the full make test /
make test-convergence sweeps, so I've left those two boxes unchecked rather than
claim more coverage than I actually have. Relying on CI for the full run.

vaibhavjindal and others added 2 commits August 7, 2026 08:45
`_triton_rope` declares `bs: tl.constexpr`, but `bs` is never referenced in
the kernel body -- it appears only in comments. Marking a dead parameter as
constexpr still forces Triton/Dynamo to specialize on it, and that breaks
torch.compile as soon as the batch dimension becomes dynamic: the value
arrives as a SymInt, and Dynamo trips an internal assertion while tracing
LigerRopeFunction through the autograd_function_apply HOP:

    assert subgraph_vt.is_tensor() or isinstance(subgraph_vt, SymNodeVariable)
      torch/_dynamo/variables/higher_order_ops.py:325

The failure only surfaced on the *second* distinct input shape, since the
first call compiles statically and the second triggers automatic dynamic
shapes -- which made it look like an upstream torch bug rather than a kernel
signature issue.

Varying seq_len alone was always fine, because `sl` is an ordinary runtime
arg; only the constexpr batch size was fatal:

                                        before      after
    vary seq_len only (auto-dynamic)      OK          OK
    vary batch only  (auto-dynamic)      FAIL         OK
    vary batch, dynamic=False             OK          OK

Dropping the constexpr annotation is sufficient. Qwen3-0.6B now compiles
under a plain `torch.compile(model)` across batch sizes 1/4/8 and sequence
lengths 512/1024/2048; previously that required a `dynamic=False` workaround.
Explicit `torch._dynamo.mark_dynamic` on the batch and sequence dims also
works now.

The parameter is kept (rather than removed) so the kernel's call signature
and the `q size: (bsz, ...)` documentation stay intact.

Note `dynamic=True` remains unsupported: it additionally marks head_dim and
the head counts dynamic, which cannot work while block sizing is derived from
`triton.next_power_of_2(head_dim)` at launch time. Those dims are fixed by
model architecture in practice, so this is not the case that matters.

Behaviour is unchanged on the eager path, and compiled output and gradients
are bit-identical to eager (max abs diff 0.0), as expected for a parameter
the kernel never reads.

Co-authored-by: Copilot <[email protected]>
@vaibhavjindal
vaibhavjindal marked this pull request as draft August 7, 2026 08:48
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.

1 participant