Skip to content

[Fix] Gate decode shortcuts and validate dims across layers - #1203

Open
zhiyuan1i wants to merge 5 commits into
mainfrom
fix/cleanup-0830
Open

[Fix] Gate decode shortcuts and validate dims across layers#1203
zhiyuan1i wants to merge 5 commits into
mainfrom
fix/cleanup-0830

Conversation

@zhiyuan1i

Copy link
Copy Markdown
Collaborator

Summary

Batch of small, independent safety fixes found by auditing the codebase for the same bug classes as recent community fix PRs:

  • Decode shortcuts misfire on packed varlen (short_conv.py, ops/rwkv7/fused_recurrent.py, ops/gsa/fused_recurrent.py): shape-equality decode checks (B*T==N, T==1) are wrong when a packed batch has zero-length or multi-token sequences — short_conv misaligns tokens to cache rows, rwkv7 reads/writes out of bounds, GSA treats a packed batch as dense. Gate shortcuts on every sequence having exactly one token.
  • Expanded dims not validated at layer construction (7 layers + mom/raven/rwkv7): int(hidden_size * expand) truncates fp-representable integers (50*0.58→28) and silently uses non-integer dims. Round first and reject true non-integers with math.isclose, and assert divisibility for head dims — the convention already used in abc/raven/kda layers. Also fixes raven's false reject of valid expand values (isclose was checked after int-truncation).
  • Mamba2 num_heads % n_groups: fail fast instead of a cryptic late reshape error in decode.
  • HGRN2 state_size: implement as input_dim * expand_ratio (GLA recurrent state shape).
  • raven seqlen max: compute only when RoPE is used (default path skips two device syncs).

Test plan

  • Unit tests added/modified: tests/layers/test_layer_dim_validation.py (17 constructor-validation cases, CPU), tests/modules/test_conv.py::test_conv_varlen_decode_detection_with_zero_len_seq
  • Dependent tests run: tests/layers/test_layer_dim_validation.py 17/17 pass locally; ruff and compileall clean
  • Varlen / CP / model tests: the short_conv test covers a packed varlen batch with a zero-length sequence and cache; GPU-dependent suites run in CI

Benchmark / NCU (kernel changes only)

  • N/A — no kernel code changed (validation guards, decode-shortcut gating, one sync-avoidance refactor)

Breaking changes

  • Invalid layer configs that were silently truncated or floored now fail loudly at construction; valid configs are bit-identical. The GSA inference shortcut no longer applies to packed varlen inputs (previously wrong for them); dense decode is unchanged.

Checklist

  • I have read CONTRIBUTING.md and follow its conventions (code style, docstrings, commit prefixes).
  • I have read AGENTS.md and, where my change matches its scope, the relevant skill under .agents/skills.
  • Dependent tests pass locally or in CI, and new behavior is covered by tests where applicable (tick as N/A for changes with no testable code, e.g. docs-only).
  • Kernel changes include same-hardware before/after benchmark numbers, dense + varlen where applicable (tick as N/A when no kernel code changed).
  • This PR is minor/cosmetic-only (typo, formatting, style-only tweaks) — tick only if it is, and justify below.

Shape-equality decode shortcuts (B*T==N, T==1) misfire on packed varlen
batches that contain zero-length or multi-token sequences: short_conv's
step() ignores cu_seqlens and misaligns tokens to cache rows, rwkv7's
decode branch reads/writes out of bounds, and GSA's inference shortcut
treats a packed batch as dense. Gate the shortcuts on every sequence
having exactly one token (dense T==1 unchanged).
int(hidden_size * expand) truncates floating-point representations of
integers (50 * 0.58 -> 28 instead of 29) and silently uses non-integer
dims. Round first and reject true non-integers with math.isclose, and
assert divisibility for head dims, matching the convention already used
in abc/raven/kda layers. Also fixes raven's false reject of valid
expand values (it checked isclose after int-truncation).
A non-divisible n_groups previously failed late with a cryptic reshape
error in the decode path.
The GLA recurrent state is [B, H, head_f_dim, head_i_dim], i.e.
input_dim * expand_ratio per sequence.
Both max computations (with their device syncs) only feed rotary, so
skip them entirely in the default use_rope=False path.
@zhiyuan1i zhiyuan1i added the bug Something isn't working label Aug 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant