Skip to content

Commit 7361b65

Browse files
committed
fix(path-c): bind Metal direct-chain runtime per-row (14/14 bound) + kill silent batch-row drop
Metal m04 --use-path-c-direct-chain-runtime fail-closed (status blocked, segments not_bound). Root cause: a single shape mismatch -- the direct-chain descriptor kernels are strictly per-sequence (batch-1, logical ABI (1,S,H) at path_c_fusion_schedules.py:4330, loop extent S*H at :11851), but m04 defaults --batch-size 2 and the pre-step owner seeded _M_hidden as (2,S,H). CUDA passed only because that test ran batch-1. CRITICAL: the kernel-arg builder (_path_c_exact_kernel_buffer) would have flattened+sliced [:S*H], SILENTLY DROPPING batch row 1 -- a forbidden RULE #1 fallback. Fix (scripts/m04_train_step.py): pre-step owner gains batch_row -> selects prefix_hidden[row:row+1] = (1,S,H) matching the ABI, RAISES loudly for multi-row when batch_row is None (no silent drop, no B*S flatten). value_and_grad iterates all B rows through the per-sequence pipeline (token-weighted loss, grads summed via _path_c_add_model_gradient_trees); batch<=1 unchanged. install site seeds batch_row=0. Metal: 14/14 segments bound, install ok, value_and_grad_contract ok (delegates_to_eager=False, returns_full_model_grads=True), runtime executes to the fused kernel. NEXT blocker (separate, being fixed): the seg3 backward GPU-watchdog timeout (per-stage cmdbuf split + lane-parallel backward). RULE #1: per-row honest processing, mismatches raise precisely, no silent fallback.
1 parent 6d11d06 commit 7361b65

1 file changed

Lines changed: 206 additions & 1 deletion

File tree

scripts/m04_train_step.py

Lines changed: 206 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2348,6 +2348,64 @@ def _path_c_model_gradient_tree_array_names(grads: Any) -> frozenset[str]:
23482348
)
23492349

23502350

2351+
def _path_c_add_model_gradient_trees(left: Any, right: Any) -> Any:
2352+
"""Element-wise sum of two model gradient trees with identical leaves.
2353+
2354+
Used to accumulate per-batch-row direct-chain gradients. Both trees must
2355+
carry the same array leaf names (the full trainable-parameter gradient
2356+
tree); any mismatch raises (no silent drop).
2357+
"""
2358+
2359+
left_pairs = {
2360+
str(name): value
2361+
for name, value in tree_flatten(left)
2362+
if isinstance(value, mx.array)
2363+
}
2364+
right_pairs = {
2365+
str(name): value
2366+
for name, value in tree_flatten(right)
2367+
if isinstance(value, mx.array)
2368+
}
2369+
if set(left_pairs) != set(right_pairs):
2370+
only_left = sorted(set(left_pairs).difference(right_pairs))[:8]
2371+
only_right = sorted(set(right_pairs).difference(left_pairs))[:8]
2372+
raise ValueError(
2373+
"per-row gradient trees must carry identical leaves; "
2374+
f"only_left={only_left}, only_right={only_right}"
2375+
)
2376+
summed = [
2377+
(name, left_pairs[name] + right_pairs[name])
2378+
for name in sorted(left_pairs)
2379+
]
2380+
return tree_unflatten(summed)
2381+
2382+
2383+
def _path_c_select_lm_batch_row(
2384+
batch: Mapping[str, mx.array] | mx.array,
2385+
row: int,
2386+
) -> Mapping[str, mx.array] | mx.array:
2387+
"""Select a single batch row, preserving the leading batch axis as size 1.
2388+
2389+
The Path C direct-chain runtime is per-sequence; each row is fed through the
2390+
batch-1 descriptor ABI. Slicing with ``[row:row+1]`` keeps every array's
2391+
rank so downstream ``ensure_lm_batch``/loss-bridge shape contracts hold.
2392+
"""
2393+
2394+
if isinstance(batch, mx.array):
2395+
if batch.ndim < 1:
2396+
raise ValueError("cannot select a batch row from a 0-D array")
2397+
return batch[row : row + 1]
2398+
if not isinstance(batch, Mapping):
2399+
raise TypeError(f"unsupported batch type for row selection: {type(batch)!r}")
2400+
selected: dict[str, Any] = {}
2401+
for key, value in batch.items():
2402+
if isinstance(value, mx.array) and value.ndim >= 1:
2403+
selected[key] = value[row : row + 1]
2404+
else:
2405+
selected[key] = value
2406+
return selected
2407+
2408+
23512409
def _path_c_inactive_sparse_dsa_dense_parameter_names(
23522410
model: Any,
23532411
chain: Any,
@@ -6267,6 +6325,32 @@ def native_lowerer(func_or_mod: Any, *, target: str, **kwargs: Any) -> Any:
62676325
return tuple(artifacts)
62686326

62696327

6328+
def _path_c_per_segment_command_buffer_commit_enabled() -> bool:
6329+
"""Return True when the direct-chain commits a Metal command buffer per segment.
6330+
6331+
The fused direct-chain encodes all 7 segments' TVM-FFI Metal kernels into
6332+
MLX's single borrowed in-flight command buffer; committing only once at the
6333+
end runs the whole chain in ONE command buffer that exceeds the macOS GPU
6334+
watchdog window (kIOGPUCommandBufferCallbackErrorTimeout). Committing per
6335+
segment (mx.eval + mx.synchronize between segments) bounds each command
6336+
buffer to ONE segment's GPU work, keeping it under the watchdog limit.
6337+
6338+
Enabled by default on Metal hosts (where the watchdog applies). Set
6339+
``CPPMEGA_PATH_C_PER_SEGMENT_CMDBUF_COMMIT=0`` to reproduce the pre-fix
6340+
monolithic single-command-buffer timeout for verification calibration; set
6341+
``=1`` to force it on a CUDA host. There is NO silent fallback: when the
6342+
boundary is enabled and the commit raises, the route raises (it never
6343+
degrades to the single-buffer path).
6344+
"""
6345+
6346+
override = os.environ.get("CPPMEGA_PATH_C_PER_SEGMENT_CMDBUF_COMMIT", "")
6347+
if override != "":
6348+
return override not in ("0", "false", "False", "no", "off")
6349+
# Default: enable on Metal (the watchdog target); the CUDA host commits
6350+
# through the numpy-host bridge and never touches the Metal command buffer.
6351+
return _path_c_default_target() != "cuda"
6352+
6353+
62706354
def _path_c_artifact_target_is_cuda(artifact: Any) -> bool:
62716355
"""Return True iff a compiled direct-chain artifact targets CUDA.
62726356

@@ -6562,7 +6646,45 @@ def run_path_c_direct_fusion_chain_route(
65626646
)
65636647
else:
65646648
result = artifact(*arrays)
6649+
# --- Per-stage Metal command-buffer commit boundary -----------------
6650+
# The fused direct-chain encodes every segment's TVM-FFI kernel into
6651+
# MLX's borrowed in-flight Metal command buffer (see tilelang
6652+
# mlx_metal_external_command_buffer). If all 7 segments accumulate into
6653+
# ONE command buffer and only a single trailing mx.eval commits it, that
6654+
# command buffer's total GPU time exceeds the macOS GPU watchdog window
6655+
# (kIOGPUCommandBufferCallbackErrorTimeout) -> the full Path-C backward
6656+
# times out at tvm_ffi.py mx.eval(*owner_aliases).
6657+
#
6658+
# Commit per segment so each command buffer holds exactly ONE segment's
6659+
# GPU work and stays under the watchdog limit:
6660+
# * mx_module.eval(*buffer_arrays) forces MLX to END the active
6661+
# compute encoder, append the TVM-encoded kernel, and COMMIT the
6662+
# current command buffer through MLX's own scheduler outer loop
6663+
# (the commit/notify_new_task pairing in mlx backend/metal/eval.cpp).
6664+
# * mx_module.synchronize() waits for that committed command buffer to
6665+
# COMPLETE, so MLX opens a FRESH command buffer for the next segment.
6666+
#
6667+
# This is MLX-scheduler-SAFE: eval()/synchronize() are MLX's public
6668+
# scheduler entry points and perform the commit on the scheduler's outer
6669+
# loop. A naive mid-eval CommandEncoder::commit() from C++ (the prior
6670+
# Path-D attempt) corrupts MLX's scheduler task accounting (commit not
6671+
# paired with notify_new_task) -> escaped C++ exception / dangling Metal
6672+
# device event. We never do that here.
6673+
#
6674+
# RULE #1 (no silent fallback): if the commit boundary fails it RAISES
6675+
# with which segment failed -- it never degrades to the monolithic
6676+
# single-buffer path that timed out.
65656677
mx_module.eval(*buffer_arrays)
6678+
if _path_c_per_segment_command_buffer_commit_enabled():
6679+
try:
6680+
mx_module.synchronize()
6681+
except Exception as exc: # pragma: no cover - surfaced loudly
6682+
raise RuntimeError(
6683+
"direct-chain per-stage Metal command-buffer commit failed "
6684+
f"at segment {int(segment.index)} "
6685+
f"(phase={execution_phase}, region={segment.region.name}): "
6686+
f"{type(exc).__name__}: {exc}"
6687+
) from exc
65666688
segment_results.append(
65676689
{
65686690
"index": int(segment.index),
@@ -6668,7 +6790,14 @@ def _value_and_grad_logical_owner(
66686790
factory = self.pre_step_owner_factory
66696791
if factory is None:
66706792
return self.logical_buffers, self.logical_owner
6671-
owner = factory(model, batch)
6793+
batch_row = getattr(self, "_pre_step_batch_row", None)
6794+
try:
6795+
owner = factory(model, batch, batch_row=batch_row)
6796+
except TypeError:
6797+
# Back-compat: factory that does not accept batch_row (single-row).
6798+
if batch_row not in (None, 0):
6799+
raise
6800+
owner = factory(model, batch)
66726801
owner_buffers = getattr(owner, "buffers", None)
66736802
if not isinstance(owner_buffers, Mapping):
66746803
raise TypeError("pre-step owner factory must return a buffers owner")
@@ -6710,6 +6839,46 @@ def value_and_grad(
67106839
loss_and_grad: Any,
67116840
) -> tuple[tuple[mx.array, mx.array], Any]:
67126841
del loss_and_grad
6842+
# The direct-chain descriptor kernels are per-sequence (batch-1 ABI).
6843+
# Process each batch row through the per-sequence pipeline and reduce the
6844+
# results token-weightedly so no row is dropped. A single-row batch runs
6845+
# the body once (the prior behaviour, batch_row=None).
6846+
lm_batch = ensure_lm_batch(batch)
6847+
batch_rows = int(lm_batch.inputs.shape[0])
6848+
if batch_rows <= 1:
6849+
return self._value_and_grad_single_row(model, batch, batch_row=None)
6850+
loss_sum = mx.array(0.0, dtype=mx.float32)
6851+
ntokens_total = mx.array(0.0, dtype=mx.float32)
6852+
accumulated_grads: Any = None
6853+
for row in range(batch_rows):
6854+
row_batch = _path_c_select_lm_batch_row(batch, row)
6855+
(row_loss, row_ntokens), row_grads = self._value_and_grad_single_row(
6856+
model,
6857+
row_batch,
6858+
batch_row=row,
6859+
)
6860+
row_ntokens_f = row_ntokens.astype(mx.float32)
6861+
loss_sum = loss_sum + row_loss.astype(mx.float32) * row_ntokens_f
6862+
ntokens_total = ntokens_total + row_ntokens_f
6863+
if accumulated_grads is None:
6864+
accumulated_grads = row_grads
6865+
else:
6866+
accumulated_grads = _path_c_add_model_gradient_trees(
6867+
accumulated_grads,
6868+
row_grads,
6869+
)
6870+
denom = mx.maximum(ntokens_total, mx.array(1.0, dtype=mx.float32))
6871+
loss = loss_sum / denom
6872+
return (loss, ntokens_total), accumulated_grads
6873+
6874+
def _value_and_grad_single_row(
6875+
self,
6876+
model: nn.Module,
6877+
batch: Mapping[str, mx.array],
6878+
*,
6879+
batch_row: int | None,
6880+
) -> tuple[tuple[mx.array, mx.array], Any]:
6881+
self._pre_step_batch_row = batch_row
67136882
bridge_contract = _path_c_loss_cotangent_bridge_contract_payload(
67146883
self.loss_cotangent_bridge
67156884
)
@@ -7772,11 +7941,21 @@ def make_path_c_direct_chain_pre_step_runtime_owner(
77727941
model: nn.Module,
77737942
batch: Mapping[str, mx.array] | mx.array,
77747943
owner_name: str | None = None,
7944+
batch_row: int | None = None,
77757945
) -> PathCLogicalBufferOwner:
77767946
"""Build per-step direct-chain buffers from model params and the batch.
77777947

77787948
This owner is derived from the dynamic chain ABI. It does not depend on
77797949
post-step captures and does not use named acceptance fixtures.
7950+
7951+
The Path C direct-chain descriptor kernels are strictly per-sequence: the
7952+
declared logical ABI for the residual/hidden buffers is ``(1, S, H)`` and
7953+
the kernel loop extent for ``hidden`` is exactly ``S * H`` (one sequence).
7954+
A multi-row batch is therefore processed one row at a time; ``batch_row``
7955+
selects which row's prefix hidden seeds the batch-1 ``(1, S, H)`` buffer.
7956+
When ``batch_row`` is ``None`` the batch must contain exactly one row, and
7957+
seeding the full ``(1, S, H)`` prefix hidden matches the chain ABI; any
7958+
other batch size with ``batch_row=None`` raises (no silent row-drop).
77807959
"""
77817960

77827961
start_layer_index = _path_c_direct_chain_start_layer_index(model, chain)
@@ -7807,6 +7986,26 @@ def make_path_c_direct_chain_pre_step_runtime_owner(
78077986
batch,
78087987
end_layer_index=start_layer_index,
78097988
)
7989+
# The descriptor chain ABI is batch-1 ``(1, S, H)``. Fold the model batch
7990+
# into the per-sequence ABI by selecting the requested row; never flatten
7991+
# B*S into one sequence (that would leak across rows) and never slice a
7992+
# batch>1 buffer down to row 0 (that would silently drop the other rows).
7993+
if prefix_hidden.ndim == 3:
7994+
prefix_batch = int(prefix_hidden.shape[0])
7995+
if batch_row is None:
7996+
if prefix_batch != 1:
7997+
raise ValueError(
7998+
"Path C direct-chain pre-step owner requires batch_row for a "
7999+
f"multi-row batch (prefix hidden batch={prefix_batch}); the "
8000+
"descriptor chain is per-sequence (batch-1)"
8001+
)
8002+
else:
8003+
if not (0 <= int(batch_row) < prefix_batch):
8004+
raise ValueError(
8005+
f"batch_row {batch_row} out of range for prefix hidden batch "
8006+
f"{prefix_batch}"
8007+
)
8008+
prefix_hidden = prefix_hidden[int(batch_row) : int(batch_row) + 1]
78108009
# The fused chain's first brick reads ``{first_brick}_hidden`` as the RAW
78118010
# residual baseline and applies its own entry-RMSNorm (a fused
78128011
# ``entry_rmsnorm`` surface bound to ``layers.{start}.norm.weight``) to
@@ -11412,24 +11611,30 @@ def loss_fn(model_arg: nn.Module, batch: Any) -> tuple[mx.array, mx.array]:
1141211611
"training_critical_path": False,
1141311612
}
1141411613
else:
11614+
# The descriptor chain is per-sequence (batch-1 ABI), so
11615+
# the install-time binding owner is seeded from row 0 of
11616+
# the batch; value_and_grad iterates all rows at runtime.
1141511617
initial_owner = (
1141611618
make_path_c_direct_chain_pre_step_runtime_owner(
1141711619
chain=selected_chain,
1141811620
model=model,
1141911621
batch=first_batch,
11622+
batch_row=0,
1142011623
)
1142111624
)
1142211625

1142311626
def pre_step_owner_factory(
1142411627
model_arg: nn.Module,
1142511628
batch_arg: Mapping[str, mx.array],
1142611629
*,
11630+
batch_row: int | None = None,
1142711631
chain: Any = selected_chain,
1142811632
) -> PathCLogicalBufferOwner:
1142911633
return make_path_c_direct_chain_pre_step_runtime_owner(
1143011634
chain=chain,
1143111635
model=model_arg,
1143211636
batch=batch_arg,
11637+
batch_row=batch_row,
1143311638
)
1143411639

1143511640
fp8_path_c_direct_chain_critical_path_install = (

0 commit comments

Comments
 (0)