Skip to content

Failed PR - #264

Closed
seemanne wants to merge 4 commits into
ggml-org:masterfrom
seemanne:publish-package
Closed

Failed PR#264
seemanne wants to merge 4 commits into
ggml-org:masterfrom
seemanne:publish-package

Conversation

@seemanne

@seemanne seemanne commented Mar 18, 2023

Copy link
Copy Markdown

This PR makes the changes required to push the model to pypi, where it now resides

Edit: sorry for the pings lads this was supposed to go into my fork.

@seemanne seemanne closed this Mar 18, 2023
@seemanne seemanne changed the title Publish package Failed PR Mar 18, 2023
@seemanne
seemanne deleted the publish-package branch March 18, 2023 13:39
Deadsg pushed a commit to Deadsg/llama.cpp that referenced this pull request Dec 19, 2023
fix "missing 1 required positional argument: 'min_keep'"
phuongncn pushed a commit to phuongncn/llama.cpp-gx10-dgx-sparks-deepseekv4 that referenced this pull request Apr 28, 2026
Co-authored-by: Iwan Kawrakow <iwan.kawrakow@gmail.com>
Defilan added a commit to Defilan/llama.cpp that referenced this pull request Aug 4, 2026
Wires the pieces together, so MoE prompt processing for a TQ3_1S/TQ4_1S model
runs the rotated matmul instead of staging every expert tensor as f16.

The getter cases and the rotate orchestration land in one commit on purpose:
either alone runs the rotated matmul against an unrotated activation and
returns silently wrong numbers.

Flow when tq_rotate is set:
  1. force staging, so d_Y is ctx->prealloc_y and never aliases src1
  2. contiguous f32 copy src1 -> prealloc_y
  3. barrier, then tq_rotate_act in place on prealloc_y
  4. mul_mm_id reads the rotated f32 copy against centroid*scale weights

Details that have to agree or the result is quietly wrong:

- `tq_rotate` requires `mmp != nullptr && !x_non_contig`, i.e. that the TQ
  pipeline was really selected. On the f16 dequant fallback the weights are
  true weights and must NOT see a rotated activation.
- Staging is forced even for a contiguous f32 src1. Otherwise y_f32_kernel is
  true, qy_needs_dequant is false, and d_Y aliases the graph's own tensor --
  the hazard Metal's in-place rotate/un-rotate has to work around. The
  "not implemented" assert is relaxed for exactly this case.
- effective_src1_type and y_sz both switch to f32, matching what the rotate
  writes. The staging pipeline is an f32 cpy, not the usual f16 one: the
  butterfly is 5 rounds of adds and f16 would lose precision quantization did
  not.
- The prealloc_y reuse cache is keyed on the ROTATE pipeline pointer rather
  than the cpy pipeline. prealloc_y now holds rotated data, so a later
  non-rotated consumer of the same src1 must not reuse it; keying this way
  forces a re-stage for anyone else.
- Only the ne11*ne12*ne13 rows the copy wrote are rotated; the padded_n rows
  above ne11 are left alone, as in the f16 staging path.
- ggml_vk_get_mul_mat_mat_id_pipeline() ends in GGML_ASSERT(support_fp32acc),
  so listing TQ in its switch would ABORT on a coopmat2 device, where these
  pipelines are deliberately not created. It now returns nullptr when both acc
  variants are empty, falling back to the f16 dequant path.

Not yet done: the maxStorageBufferRange gate in supports_op still rejects large
expert tensors on the old premise that prompt processing must stage them as
f16. With this path that premise no longer holds, but relaxing it is a separate
commit pending review of ggml-org#264.

Assisted-by: Claude
satindergrewal added a commit to satindergrewal/llama.cpp that referenced this pull request Aug 12, 2026
…ed by 8 rows; gap 2.91x -> 2.60x

The Q-block increment from the champion transfer list: simd group s owns query row
tgpig.x*nsg + s; each KV block's K and V tiles are staged ONCE into threadgroup memory by
all threads cooperatively and read by every row -- the per-token global traffic that made
paged prefill a bandwidth problem is now amortized across 8 rows. Barriers are per BLOCK
and threadgroup-uniform by construction (bounds derive from tgpig only); per-row token
loops inside a staged block may diverge, which is safe because no barrier lives inside
them. Tail rows past n_tokens_total CLAMP instead of returning -- a non-uniform early
return would deadlock the staging barriers. Cross-seq packs (rare boundary) fall back to
the proven per-row unshared walk, so correctness is exact everywhere.

nsg swept, every point 12/12-gated BEFORE its wall (ms, 1,523-token prefill):
  2 -> 3,565   4 -> 3,078   8 -> 2,940   16 -> 3,247
Turnover at 16; 8 ships. The sweep also caught a real defect class: nsg lives in THREE
coupled places (kargs, nth, grid divisor) and changing one produced a kernel where half
the rows were never computed -- caught by the gate, wall discarded as invalid.

DURABLE (n=3, one binary, answers IDENTICAL, results/metal-qblock-gate-* + baseline-1645):
  prefill  2,936-2,948 ms  vs static 1,131-1,140  = 2.60x  (was 2.91x; scalar start 6.1x)
  decode   63.2-63.4 tok/s unchanged (1.60x off static)

Credits: Q-block/tile structure from llama.cpp's own kernel_flash_attn_ext
(ggml-metal.metal, Georgi Gaessler/ggml authors); staged-tile pattern cross-checked
against MLX steel_attention.h (Apple, MIT); wide-tile amortization principle measured
independently in antirez/ds4 PR ggml-org#264; block-granular gather from vLLM PagedAttention.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ckti pushed a commit to ckti/llama.cpp-consolidated that referenced this pull request Aug 22, 2026
vulkan : add mul_mat_vec_id for the TurboQuant weight types
ckti pushed a commit to ckti/llama.cpp-consolidated that referenced this pull request Aug 22, 2026
Wires the pieces together, so MoE prompt processing for a TQ3_1S/TQ4_1S model
runs the rotated matmul instead of staging every expert tensor as f16.

The getter cases and the rotate orchestration land in one commit on purpose:
either alone runs the rotated matmul against an unrotated activation and
returns silently wrong numbers.

Flow when tq_rotate is set:
  1. force staging, so d_Y is ctx->prealloc_y and never aliases src1
  2. contiguous f32 copy src1 -> prealloc_y
  3. barrier, then tq_rotate_act in place on prealloc_y
  4. mul_mm_id reads the rotated f32 copy against centroid*scale weights

Details that have to agree or the result is quietly wrong:

- `tq_rotate` requires `mmp != nullptr && !x_non_contig`, i.e. that the TQ
  pipeline was really selected. On the f16 dequant fallback the weights are
  true weights and must NOT see a rotated activation.
- Staging is forced even for a contiguous f32 src1. Otherwise y_f32_kernel is
  true, qy_needs_dequant is false, and d_Y aliases the graph's own tensor --
  the hazard Metal's in-place rotate/un-rotate has to work around. The
  "not implemented" assert is relaxed for exactly this case.
- effective_src1_type and y_sz both switch to f32, matching what the rotate
  writes. The staging pipeline is an f32 cpy, not the usual f16 one: the
  butterfly is 5 rounds of adds and f16 would lose precision quantization did
  not.
- The prealloc_y reuse cache is keyed on the ROTATE pipeline pointer rather
  than the cpy pipeline. prealloc_y now holds rotated data, so a later
  non-rotated consumer of the same src1 must not reuse it; keying this way
  forces a re-stage for anyone else.
- Only the ne11*ne12*ne13 rows the copy wrote are rotated; the padded_n rows
  above ne11 are left alone, as in the f16 staging path.
- ggml_vk_get_mul_mat_mat_id_pipeline() ends in GGML_ASSERT(support_fp32acc),
  so listing TQ in its switch would ABORT on a coopmat2 device, where these
  pipelines are deliberately not created. It now returns nullptr when both acc
  variants are empty, falling back to the f16 dequant path.

Not yet done: the maxStorageBufferRange gate in supports_op still rejects large
expert tensors on the old premise that prompt processing must stage them as
f16. With this path that premise no longer holds, but relaxing it is a separate
commit pending review of ggml-org#264.

Assisted-by: Claude
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