Skip to content

Commit d8607e9

Browse files
author
Codex
committed
Path C blocker 1: fix sparse_mla_fp8_apply softmax score-max indent
The descriptor emitter for the row-phased sparse-MLA FP8 apply body placed the per-key score-max update and score-weight accumulation one indent level too deep, which Python parses as the body of the else-branch of the preceding 'if sparse_index >= 0 ... < seq:' guard. At runtime this left score_max at -inf for every valid sparse key, which collapsed the softmax (sumexp -> +inf, context_accum -> 0) and produced attention_out identically zero end-to-end. The brick backward then propagated those zeros into every parameter grad slot fed by the apply (q/kv proj weight+bias, out proj weight+bias, etc.) and the suffix-loss cotangent bridge sat on top of all-zero activations. The fix realigns four code-emit sites in _append_row_phased_sparse_mla_fp8_apply_body so the score-max update, the score-weight exponential, the score-weight cache, and the lse-only sumexp accumulation are siblings of the per-key if/else (i.e. live in the enclosing 'for k_top:' loop body). End-to-end launch on the local_gb10_quarter tiny smoke config now produces attention_out with 8192/8192 non-zero, all-finite entries (sum|.| ~= 4.6e7) instead of all zeros. Adds tests/test_path_c_sparse_mla_apply_emit_indent.py with three structural assertions that regression-lock the indent invariants against the rendered tiny train-block PrimFunc: - score-max update lives at the same indent as the matching 'if sparse_index >= 0 ...' guard - score-weight T.exp(...) accumulator lives at that indent - lse-only sumexp += T.exp(score_accum - score_max) lives at that indent Verification: - ruff clean on the changed file and the new test. - pyright clean on the changed file and the new test. - pytest tests/test_path_c_sparse_mla_apply_emit_indent.py -> 3 passed - pytest tests/test_path_c_fusion_ir.py tests/test_path_c_fusion_templates.py -> 111 passed - pytest tests/test_path_c_physical_abi.py tests/test_path_c_fused_plus_eager_runtime.py tests/test_path_c_fused_suffix_custom_function.py tests/test_hybrid_lm_path_c_physical_abi_bank_owner.py tests/test_compiled_train.py -> 91 passed
1 parent bd7534d commit d8607e9

2 files changed

Lines changed: 160 additions & 8 deletions

File tree

cppmega_mlx/runtime/path_c_fusion_schedules.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5099,8 +5099,8 @@ def append_score_for_current_head(
50995099
kv_head_expr=kv_head,
51005100
use_cached_sparse_index=True,
51015101
)
5102-
body.append(f"{indent * 6}if {score_accum}[0] > {score_max}[0]:")
5103-
body.append(f"{indent * 7}{score_max}[0] = {score_accum}[0]")
5102+
body.append(f"{indent * 5}if {score_accum}[0] > {score_max}[0]:")
5103+
body.append(f"{indent * 6}{score_max}[0] = {score_accum}[0]")
51045104
body.append(f"{indent * 4}if {sink_enabled}[0] != 0.0:")
51055105
body.append(f"{indent * 5}if {sinks} > {score_max}[0]:")
51065106
body.append(f"{indent * 6}{score_max}[0] = {sinks}")
@@ -5113,13 +5113,13 @@ def append_score_for_current_head(
51135113
use_cached_sparse_index=True,
51145114
)
51155115
body.append(
5116-
f"{indent * 6}{score_weight}[0] = "
5116+
f"{indent * 5}{score_weight}[0] = "
51175117
f"T.exp({score_accum}[0] - {score_max}[0])"
51185118
)
51195119
body.append(
5120-
f"{indent * 6}{score_weights}[{k_top}] = {score_weight}[0]"
5120+
f"{indent * 5}{score_weights}[{k_top}] = {score_weight}[0]"
51215121
)
5122-
body.append(f"{indent * 6}{sumexp}[0] = {sumexp}[0] + {score_weight}[0]")
5122+
body.append(f"{indent * 5}{sumexp}[0] = {sumexp}[0] + {score_weight}[0]")
51235123
body.append(f"{indent * 4}if {sink_enabled}[0] != 0.0:")
51245124
body.append(
51255125
f"{indent * 5}{sumexp}[0] = {sumexp}[0] + "
@@ -5239,8 +5239,8 @@ def append_score_for_current_head(
52395239
head_expr=q_head,
52405240
kv_head_expr=kv_head,
52415241
)
5242-
body.append(f"{indent * 6}if {score_accum}[0] > {score_max}[0]:")
5243-
body.append(f"{indent * 7}{score_max}[0] = {score_accum}[0]")
5242+
body.append(f"{indent * 5}if {score_accum}[0] > {score_max}[0]:")
5243+
body.append(f"{indent * 6}{score_max}[0] = {score_accum}[0]")
52445244
body.append(f"{indent * 4}if {sink_enabled}[0] != 0.0:")
52455245
body.append(f"{indent * 5}if {sinks} > {score_max}[0]:")
52465246
body.append(f"{indent * 6}{score_max}[0] = {sinks}")
@@ -5252,7 +5252,7 @@ def append_score_for_current_head(
52525252
kv_head_expr=kv_head,
52535253
)
52545254
body.append(
5255-
f"{indent * 6}{sumexp}[0] = {sumexp}[0] + "
5255+
f"{indent * 5}{sumexp}[0] = {sumexp}[0] + "
52565256
f"T.exp({score_accum}[0] - {score_max}[0])"
52575257
)
52585258
body.append(f"{indent * 4}if {sink_enabled}[0] != 0.0:")
Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
"""Regression tests for sparse_mla_fp8_apply emit-indent fixes.
2+
3+
These tests pin the structural invariant that the lowered PrimFunc
4+
source for the Mamba3 FP8 train block keeps the softmax score-max
5+
update statements outside the per-key ``if/else`` branch in the
6+
``sparse_mla_fp8_apply`` body. A previous version emitted the
7+
update inside the ``else:`` branch (because of an indentation mistake
8+
in :func:`cppmega_mlx.runtime.path_c_fusion_schedules._append_row_phased_sparse_mla_fp8_apply_body`),
9+
which left ``score_max`` at ``-inf`` for every valid key and produced
10+
``attention_out == 0`` end-to-end.
11+
12+
The fix changes the descriptor emit so the score-max update and the
13+
softmax-weight accumulation are siblings of the ``if/else`` block.
14+
These tests verify that:
15+
16+
1. Every ``if sparse_mla_fp8_apply_score_accum[0] > sparse_mla_fp8_apply_score_max[0]:``
17+
line in the rendered PrimFunc has the SAME indent as the matching
18+
``if sparse_mla_fp8_apply_sparse_index[0] >= 0`` line.
19+
2. Every ``sparse_mla_fp8_apply_score_weight[0] = T.exp(``
20+
line is at the SAME indent as the matching ``if sparse_index`` line
21+
(so the score weights accumulate over ALL keys, not just invalid ones).
22+
3. Every ``sparse_mla_fp8_apply_sumexp[0] = sparse_mla_fp8_apply_sumexp[0]
23+
+ T.exp(score_accum`` line is at the SAME indent as the matching
24+
``if sparse_index`` line.
25+
"""
26+
27+
from __future__ import annotations
28+
29+
import contextlib
30+
import io
31+
32+
from cppmega_mlx.recipes.model_factory import local_gb10_quarter_profile
33+
from cppmega_mlx.runtime import path_c_fusion_schedules as schedules
34+
35+
36+
def _rendered_train_block_source() -> str:
37+
profile = local_gb10_quarter_profile()
38+
tiny = profile.tiny_smoke_config()
39+
with contextlib.redirect_stderr(io.StringIO()):
40+
region = schedules._mamba3_fp8_train_acceptance_region(
41+
include_backward=True, model_config=tiny,
42+
)
43+
prim_func = schedules.mamba3_fp8_train_fusion_schedule_template(region)
44+
return str(prim_func)
45+
46+
47+
def _indent_of(line: str) -> int:
48+
"""Return the number of leading spaces."""
49+
return len(line) - len(line.lstrip(" "))
50+
51+
52+
def test_sparse_mla_apply_score_max_update_is_sibling_of_branch() -> None:
53+
"""Every ``score_max = score_accum`` update must live outside the
54+
per-key ``if sparse_index ... < seq:`` branch so that valid keys can
55+
actually update the running max. Before the fix this lived inside
56+
the ``else:`` branch and the max stayed at ``-inf`` for every valid
57+
key, producing ``attention_out`` of all zeros."""
58+
59+
src = _rendered_train_block_source()
60+
lines = src.split("\n")
61+
branch_indents: list[int] = []
62+
update_indents: list[int] = []
63+
for index, line in enumerate(lines):
64+
stripped = line.strip()
65+
if stripped.startswith(
66+
"if sparse_mla_fp8_apply_sparse_index[0] >= 0 "
67+
"and sparse_mla_fp8_apply_sparse_index[0] < "
68+
):
69+
branch_indents.append(_indent_of(line))
70+
if stripped.startswith(
71+
"if sparse_mla_fp8_apply_score_accum[0] > "
72+
"sparse_mla_fp8_apply_score_max[0]:"
73+
):
74+
update_indents.append(_indent_of(line))
75+
# The body emits the apply twice: once in the lane-strided
76+
# context-and-out-projection branch (with sinks), once in the
77+
# lse-only branch. The lse-only branch ALSO emits the score-max
78+
# update. Whether the second emit fires depends on schedule config;
79+
# we only assert when at least one update is emitted.
80+
assert update_indents, "sparse_mla_fp8_apply score-max update missing from emit"
81+
# Each score-max update must match the indent of one of the
82+
# sparse_index branches.
83+
for indent in update_indents:
84+
assert indent in branch_indents, (
85+
f"sparse_mla_fp8_apply score-max update indent {indent} "
86+
f"is not aligned to any sparse_index branch indent {branch_indents}; "
87+
"this indicates the update was emitted inside the else-branch "
88+
"(the original Blocker 1 bug) instead of as a sibling"
89+
)
90+
91+
92+
def test_sparse_mla_apply_score_weight_exp_is_sibling_of_branch() -> None:
93+
"""``score_weight[0] = T.exp(score_accum - score_max)`` must live in
94+
the ``for k_top:`` loop body, outside the per-key
95+
``if sparse_index ... < seq:`` branch. Otherwise ``score_weights``
96+
stays at 0.0 for every valid key (because the body only ran for
97+
invalid keys in the else branch), and the softmax-weighted value
98+
accumulator is identically zero."""
99+
100+
src = _rendered_train_block_source()
101+
lines = src.split("\n")
102+
branch_indents: list[int] = []
103+
exp_indents: list[int] = []
104+
for line in lines:
105+
stripped = line.strip()
106+
if stripped.startswith(
107+
"if sparse_mla_fp8_apply_sparse_index[0] >= 0 "
108+
"and sparse_mla_fp8_apply_sparse_index[0] < "
109+
):
110+
branch_indents.append(_indent_of(line))
111+
if stripped.startswith(
112+
"sparse_mla_fp8_apply_score_weight[0] = T.exp("
113+
):
114+
exp_indents.append(_indent_of(line))
115+
assert exp_indents, "sparse_mla_fp8_apply score-weight exp missing from emit"
116+
for indent in exp_indents:
117+
assert indent in branch_indents, (
118+
f"sparse_mla_fp8_apply score-weight exp indent {indent} "
119+
f"is not aligned to any sparse_index branch indent {branch_indents}"
120+
)
121+
122+
123+
def test_sparse_mla_apply_lse_sumexp_is_sibling_of_branch() -> None:
124+
"""``sumexp[0] = sumexp[0] + T.exp(score_accum - score_max)`` in the
125+
lse-only path must live in the ``for k_top:`` loop body, outside
126+
the per-key ``if sparse_index`` branch. Otherwise the lse buffer
127+
stays at ``-inf`` for valid keys."""
128+
129+
src = _rendered_train_block_source()
130+
lines = src.split("\n")
131+
branch_indents: list[int] = []
132+
sumexp_indents: list[int] = []
133+
for line in lines:
134+
stripped = line.strip()
135+
if stripped.startswith(
136+
"if sparse_mla_fp8_apply_sparse_index[0] >= 0 "
137+
"and sparse_mla_fp8_apply_sparse_index[0] < "
138+
):
139+
branch_indents.append(_indent_of(line))
140+
if stripped.startswith(
141+
"sparse_mla_fp8_apply_sumexp[0] = "
142+
"sparse_mla_fp8_apply_sumexp[0] + T.exp("
143+
"sparse_mla_fp8_apply_score_accum"
144+
):
145+
sumexp_indents.append(_indent_of(line))
146+
# The sumexp += T.exp(score_accum - score_max) form only exists
147+
# in the lse-only path; this test only asserts when it is present.
148+
for indent in sumexp_indents:
149+
assert indent in branch_indents, (
150+
f"sparse_mla_fp8_apply lse-only sumexp indent {indent} "
151+
f"is not aligned to any sparse_index branch indent {branch_indents}"
152+
)

0 commit comments

Comments
 (0)