Skip to content

cuda: bound the one-column MUL_MAT_ID mat-vec store by the expert row count - #295

Open
professorpalmer wants to merge 1 commit into
PrismML-Eng:prismfrom
professorpalmer:cuda-mmvq-mmid-row-bound
Open

professorpalmer wants to merge 1 commit into
PrismML-Eng:prismfrom
professorpalmer:cuda-mmvq-mmid-row-bound

Conversation

@professorpalmer

Copy link
Copy Markdown

Overview

Fixes the intermittent MUL_MAT_ID failure reported on #221 (type_a=ptq1_0, n_mats=4, n_used=2, m=70, n=1, k=2048, error ~0.03 against 5e-4, on the base as well as the PR).

Cause. For one-token MUL_MAT_ID, ggml_cuda_mul_mat_vec_q puts the expert slots in channels and passes stride_col_dst = nb2 = nrows * n_used. The store guard in mul_mat_vec_q is row0 + i < stride_col_dst, so when a block holds several rows and nrows is not a multiple of that, the last block of slot s stores its out-of-range rows into the first rows of slot s + 1. Two blocks write the same addresses and whichever lands last wins, which is why the failure is intermittent. At one column a block only holds several rows with the small-K geometry (should_use_small_k); PTQ1_0 takes it at k = 2048 (16 blocks per row).

Fix. Bound the store (and the fused bias prefetch, which used the same guard) by the channel stride, which is the row count, in the one-column ids case. Other paths are unchanged.

The same guard is in ggml-org master (mmvq.cu, row0 + i < stride_col_dst); other types reach it with short K, see below.

Additional information

New test-backend-ops cases: every quantized type, one token, K = 2 blocks (forces small-K), m = 67, n_used 2 and 4. The existing generic cases use m = 512, a multiple of every rows-per-block, so they could not hit the tail.

RTX 4070 (sm_89), CUDA 13, Windows, -b CUDA0:

before after
#221's PTQ1_0 case, 300 runs 67 failed 0
new m = 67 cases, 30 sweeps q4_0, q5_0, q5_1, iq1_s, iq1_m, iq2_xxs, iq2_xs, iq2_s, iq4_xs, nvfp4 fail (most 22-30 of 30 for n_used=4) 0

Full suites on the fixed build, default and GGML_CUDA_BATCH_INVARIANT=1: MUL_MAT_ID 1085/1085, MUL_MAT 1516/1516, FLASH_ATTN_EXT 2994/2994, GATED_DELTA_NET, GET_ROWS all pass.

Requirements

  • I have read and agree with the contributing guidelines
  • AI usage disclosure: YES - the repro loop, the trace to the store guard, the patch and the test cases were done with Claude Code (Claude Opus 5.5).

🤖 Generated with Claude Code

… count

With ids and one token the expert slots are channels and stride_col_dst is nrows * n_used, so with small-K
geometry (several rows per block) the last block of a slot stored its out-of-range rows into the next
slot's first rows. Which write landed last decided the result, so the error was intermittent (PTQ1_0
m=70 k=2048: 67/300 runs failed on a 4070). The fused bias prefetch used the same bound.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant