Skip to content

ggml : add ggml_conv_1d_grouped - #22833

Closed
Juste-Leo2 wants to merge 8 commits into
ggml-org:masterfrom
Juste-Leo2:Conv1dGrouped
Closed

ggml : add ggml_conv_1d_grouped#22833
Juste-Leo2 wants to merge 8 commits into
ggml-org:masterfrom
Juste-Leo2:Conv1dGrouped

Conversation

@Juste-Leo2

@Juste-Leo2 Juste-Leo2 commented May 8, 2026

Copy link
Copy Markdown
Contributor

Overview

This PR adds the ggml_conv_1d_grouped operation (sub-graph) to be used in supporting CCA (Compressed Convolutional Attention) for future support of Zyphra's models (ZAYA1).
(This is a first step towards #22776)

CCA uses a specific convolution system that doesn't seem to be currently implemented in llama.cpp.

image

Figure 1: Architecture of Compressed Convolutional Attention (CCA), extracted from the ZAYA1 technical report (arXiv:2605.05365).

Here, the added operation amounts to supporting Depthwise Conv and Headwise Conv.

To explain how it works, I made a diagram with matplotlib:

output (2)

Figure 2: Functioning of the Grouped 1D Convolution operation (example with Groups = 2).

For example, when the group is equal to 2, we have an initial splitting of the tensors with separate convolutions. Then comes a concatenation to get the final tensor back.

Additional information

The code contains a small trick: the idea is to use ggml_view_3d to avoid making lots of memory copies. I took care to keep the code modifications to a minimum to make the review easier.

Here are the results obtained for the tests:

~/llama.cpp$ ./build_test/bin/test-conv-1d-grouped
Testing ggml_conv_1d_grouped

  TEST: groups=1 (standard conv1d) (IC=128 OC=256 K=3 L=32 G=1 s=1 p=0) kernel=f16
    PASS
  TEST: ZAYA1-8B exact params (IC=1280 OC=1280 K=2 L=16 G=10 s=1 p=0) kernel=f16
    PASS
  TEST: small 2 groups (IC=4 OC=4 K=2 L=8 G=2 s=1 p=0) kernel=f16
    PASS
  TEST: with padding (IC=8 OC=8 K=2 L=16 G=4 s=1 p=1) kernel=f16
    PASS
  TEST: IC != OC (IC=12 OC=6 K=3 L=10 G=3 s=1 p=0) kernel=f16
    PASS
  TEST: stride=2 (IC=8 OC=8 K=2 L=16 G=4 s=2 p=0) kernel=f16
    PASS
  TEST: longer sequence (IC=1280 OC=1280 K=2 L=128 G=10 s=1 p=0) kernel=f16
    PASS

--- bf16 ---

  TEST: groups=1 (standard conv1d) (IC=128 OC=256 K=3 L=32 G=1 s=1 p=0) kernel=bf16
    PASS
  TEST: ZAYA1-8B exact params (IC=1280 OC=1280 K=2 L=16 G=10 s=1 p=0) kernel=bf16
    PASS
  TEST: small 2 groups (IC=4 OC=4 K=2 L=8 G=2 s=1 p=0) kernel=bf16
    PASS
  TEST: with padding (IC=8 OC=8 K=2 L=16 G=4 s=1 p=1) kernel=bf16
    PASS
  TEST: IC != OC (IC=12 OC=6 K=3 L=10 G=3 s=1 p=0) kernel=bf16
    PASS
  TEST: stride=2 (IC=8 OC=8 K=2 L=16 G=4 s=2 p=0) kernel=bf16
    PASS
  TEST: longer sequence (IC=1280 OC=1280 K=2 L=128 G=10 s=1 p=0) kernel=bf16
    PASS

Result: 14 passed, 0 failed

Note: I will do my best to answer questions regarding the implementation. Currently, it uses existing llama.cpp operations, which I think is best for maintainability at the start.

Requirements

  • AI usage disclosure: YES
    • I used antigravity to understand the operation and make the diagram too.
    • AI was also used to write the code (I am not yet comfortable enough with the ggml C++ syntax) and to make the best possible optimizations.
    • To respect the coding style compared to other implementations.
    • To translate the PR (written in French by my own hands) and improve readability :)

@Juste-Leo2
Juste-Leo2 requested a review from ggerganov as a code owner May 8, 2026 10:15
@github-actions github-actions Bot added testing Everything test related ggml changes relating to the ggml tensor library for machine learning labels May 8, 2026
kmbandy added a commit to kmbandy/llama.cpp that referenced this pull request May 17, 2026
Ports PR ggml-org#22833 and PR ggml-org#23112 from ggml-org/llama.cpp onto our fork.

- ggml: add ggml_conv_1d_grouped op (depthwise + headwise conv via
  ggml_view_3d slicing, falls back to existing conv1d/dw for groups=1
  and groups=IC)
- gguf: register ZAYA arch, CCA_VAL_PROJ1/2, CCA_CONV_GRP, CCA_K_SCALE,
  RES_SCALE_HS/RES/FINAL, ZAYA_ROUTER_MLP2/4/BIASES/EDA_SCALE tensors
- src: add llama_model_zaya with alternating CCA (even) and MoE (odd)
  layers; residual scaling at every layer and final norm
- conversion/zaya.py: HF→GGUF converter for ZayaModel/ZayaForCausalLM
- Includes ggml_cont fixes for ROCm non-contiguous tensor compatibility
  and F16 cast fixes for CPU backend (from Zyphra fork review)

Markovian RSA (test-time compute method) is intentionally excluded and
will be a separate implementation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sdroege

sdroege commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

What's the status of this? It would be good to get this in to unblock #23112 (and the upcoming ZAYA1-74B).

I can confirm that this works fine on Vulkan with ZAYA1-8B.

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

What's the status of this? It would be good to get this in to unblock #23112 (and the upcoming ZAYA1-74B).

I can confirm that this works fine on Vulkan with ZAYA1-8B.

I'm waiting for PR #23660 to be merged; it's important for this operation, and I'd like to add it to the Q8 tests afterward just to make sure everything goes smoothly before reviewing this PR.

Comment thread ggml/src/ggml.c
Comment on lines +4524 to +4533
// convert BF16 kernel to F32 for mul_mat compatibility
struct ggml_tensor * a_op = ggml_reshape_2d(ctx, a, (a->ne[0] * a->ne[1]), a->ne[2]); // [OC, IC * K]
if (a->type == GGML_TYPE_BF16) {
a_op = ggml_cpy(ctx, a_op, ggml_new_tensor_2d(ctx, GGML_TYPE_F32, a_op->ne[0], a_op->ne[1]));
}

struct ggml_tensor * result =
ggml_mul_mat(ctx,
ggml_reshape_2d(ctx, im2col, im2col->ne[0], (im2col->ne[2] * im2col->ne[1])), // [N, OL, IC * K] => [N*OL, IC * K]
ggml_reshape_2d(ctx, a, (a->ne[0] * a->ne[1]), a->ne[2])); // [OC,IC, K] => [OC, IC * K]
a_op); // [OC, IC * K]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note : PR #23660 changed the im2col casting logic. To maintain mul_mat compatibility and prevent crashes with BF16 weights, the kernel must be explicitly cast to F32 here. Other convolution operations might require a similar follow-up fix.

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Rebased to include the latest merged PR!
I've also applied the necessary adjustments to ensure optimal BF16 support. Here are the updated test results:

~/llama.cpp$ ./build_test/bin/test-conv-1d-grouped
Testing ggml_conv_1d_grouped

  TEST: groups=1 (standard conv1d) (IC=128 OC=256 K=3 L=32 G=1 s=1 p=0) kernel=f16
    PASS
  TEST: ZAYA1-8B exact params (IC=1280 OC=1280 K=2 L=16 G=10 s=1 p=0) kernel=f16
    PASS
  TEST: small 2 groups (IC=4 OC=4 K=2 L=8 G=2 s=1 p=0) kernel=f16
    PASS
  TEST: with padding (IC=8 OC=8 K=2 L=16 G=4 s=1 p=1) kernel=f16
    PASS
  TEST: IC != OC (IC=12 OC=6 K=3 L=10 G=3 s=1 p=0) kernel=f16
    PASS
  TEST: stride=2 (IC=8 OC=8 K=2 L=16 G=4 s=2 p=0) kernel=f16
    PASS
  TEST: longer sequence (IC=1280 OC=1280 K=2 L=128 G=10 s=1 p=0) kernel=f16
    PASS

--- bf16 ---

  TEST: groups=1 (standard conv1d) (IC=128 OC=256 K=3 L=32 G=1 s=1 p=0) kernel=bf16
    PASS
  TEST: ZAYA1-8B exact params (IC=1280 OC=1280 K=2 L=16 G=10 s=1 p=0) kernel=bf16
    PASS
  TEST: small 2 groups (IC=4 OC=4 K=2 L=8 G=2 s=1 p=0) kernel=bf16
    PASS
  TEST: with padding (IC=8 OC=8 K=2 L=16 G=4 s=1 p=1) kernel=bf16
    PASS
  TEST: IC != OC (IC=12 OC=6 K=3 L=10 G=3 s=1 p=0) kernel=bf16
    PASS
  TEST: stride=2 (IC=8 OC=8 K=2 L=16 G=4 s=2 p=0) kernel=bf16
    PASS
  TEST: longer sequence (IC=1280 OC=1280 K=2 L=128 G=10 s=1 p=0) kernel=bf16
    PASS

Result: 14 passed, 0 failed

@pwilkin Since you guided me through the initial Zaya implementation and have been following the broader progress of this model, I'd love to get your thoughts on these updates. If everything looks good to you, we can ping Georgi for the merge!

@pwilkin pwilkin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a purely GGML-side alias (no backend kernels), I think there shouldn't be a problem with this, as the rationale is clear.

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Since this is a purely GGML-side alias (no backend kernels), I think there shouldn't be a problem with this, as the rationale is clear.

Agreed. Since ggml_conv_1d_grouped is currently only used by Zaya, writing dedicated backend kernels isn't really necessary for now.

@ggerganov Do these updates look good to you for a merge?

@Juste-Leo2

Juste-Leo2 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi @taronaeo,

Since we've worked together in the past and @pwilkin already approved the GGML-side alias, would you mind taking a quick look to give the second required approval?

I'm pinging you because Georgi seems quite busy right now and CISC is on vacation. Getting this merged is the next step I need to move forward with the Zaya model implementation (PR #23112).

Thanks!

@pwilkin

pwilkin commented Jul 22, 2026

Copy link
Copy Markdown
Member

@am17an could you maybe take a quick look?

@Juste-Leo2

Juste-Leo2 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Slight test failure with a max diff of 0.008 with Metal. I'll make a small commit to adjust the tolerance.

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

@pwilkin I just pushed the commit! Could you please approve the CI workflows? It should be good to go now :)

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Argh, I was too strict! I readjusted the threshold after seeing the Metal logs (diff=0.011520 vs 0.01) , could you run the CI again please @pwilkin? 😅

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Just a quick note to say that the three build errors are related to the test-recurrent-state-rollback test, which isn't related to this PR. The builds are working, and the second review is ready :)

@am17an

am17an commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Why can't this be done via broadcasting in conv_1d? mul-mat already supports broadcasting in batch dims. Also tests should be added in test-backend-ops, there is currently no way to invoke this op via any backend

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Hi @am17an, and thank you very much for your feedback!
I completely agree that it would have been really convenient to use broadcasting like with mul_mat to avoid the for loop. However, after looking into it, I think there are two technical hurdles with this approach in the current state of the code:

  • The 1D convolution goes through im2col, which strictly limits higher dimensions (GGML_ASSERT(b->ne[3] == 1)), preventing us from using the 4th dimension to handle groups.
  • Further down in ggml_conv_1d, the kernel is flattened via ggml_reshape_2d(...), which breaks the logical separation of the groups if we try to push them through that way.

1D convolution is a foundational operation here (especially for vision models), and I think it probably hasn't been touched in quite a while. To properly implement broadcasting support for it, it would likely require a separate PR to rework the core of ggml_conv_1d.
Just to clarify regarding test-backend-ops: I implemented this as a subgraph (a graph composition of existing ops) rather than a full-fledged standalone backend op, to avoid rewriting everything from scratch. I mentioned it in the description, but my PR title might have caused some confusion on that front.

Note: I took the opportunity to push a new commit adding a test for the groups == IC == OC (depthwise) case. This actually required adding a BF16 -> F32 conversion in the fallback. I am aware that it's not the most elegant, but BF16 is a bit unique. This keeps the precision intact without having to handle all the complex quantization cases right now.
Also, I updated the tests to include the batch dimension N in anticipation of future support, though for now, it must remain equal to 1.

Thanks again for your time :) , and please let me know if this explanation makes sense and justifies the current design choice!

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

Hi @am17an, just following up as this PR is blocking the draft for the Zaya model (#23112). I know you're often busy, so I'll keep it brief.

Reworking all backends to handle groups natively would be a big refactor for an op currently only used by Zaya. This workaround matches the Zaya draft and keeps the backend code completely untouched. Does this approach work for you for now?

@pwilkin, could you please re-trigger the CI when you have a chance? I added the missing case in the last commit. Thanks!

@am17an

am17an commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

If it's just for a model and it is composed of other ggml ops, you can just create a static function when you define the model

@Juste-Leo2

Copy link
Copy Markdown
Contributor Author

If it's just for a model and it is composed of other ggml ops, you can just create a static function when you define the model

@am17an, thanks for the suggestion!

I'll close this PR and implement it as a static function directly in the Zaya draft (#23112). I'll also port the BF16/F32 casting fixes for im2col over there, since #23660 didn't fully cover these specific cases.

Thanks again for your time!

@Juste-Leo2 Juste-Leo2 closed this Jul 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ggml changes relating to the ggml tensor library for machine learning testing Everything test related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants