Skip to content

QVAC-21921 ggml-speech: ACE-Step Oobleck VAE custom ops (snake, col2im_1d) — CPU + Metal#43

Open
freddy311082 wants to merge 2 commits into
speechfrom
feat/QVAC-21921-acestep-ops
Open

QVAC-21921 ggml-speech: ACE-Step Oobleck VAE custom ops (snake, col2im_1d) — CPU + Metal#43
freddy311082 wants to merge 2 commits into
speechfrom
feat/QVAC-21921-acestep-ops

Conversation

@freddy311082

Copy link
Copy Markdown

Summary

Adds the two custom ggml ops the ACE-Step Oobleck VAE / DiT graphs rely on, so
the audiogen (ACE-Step music generation) engine can run on this fork. Lands
both the CPU reference implementations and the Metal GPU kernels, so the VAE
decode runs on Apple Silicon (Metal) as well as CPU.

Part of QVAC-21921 (native ACE-Step music generation). Consumed downstream by
the audiogen-cpp engine (qvac-ext-lib-whisper.cpp) via the ggml-speech
vcpkg port, which builds from this branch.

What's in here

New ops (GGML_OP_SNAKE, GGML_OP_COL2IM_1D):

  • ggml_snake: per-channel snake activation y = x + sin^2(a*x) * inv_b
    (Oobleck VAE activations).
  • ggml_col2im_1d: scatter-add col2im for the decoder's 1D transposed convs.
  • Public API + op enum + graph plumbing, plus a tensor-name bump for the new
    op metadata.

CPU backend (commit 1): reference implementations for both ops.

Metal backend (commit 2):

  • kernel_snake_f32 and kernel_col2im_1d_f32, wired through the device
    pipeline cache, the op dispatch/encoder, and ggml_metal_device_supports_op
    (contiguous f32 in/out).
  • col2im uses a flat grid-stride so occupancy stays high even when OC is tiny
    (the final stereo transpose-conv).

Scope is additive: only the two new ops + their CPU/Metal kernels. No changes to
existing ops.

Add the two custom ggml ops ACE-Step needs, with CPU kernels:
- GGML_OP_SNAKE (snake activation used by the Oobleck VAE)
- GGML_OP_COL2IM_1D (1D col2im for transposed conv upsampling)

Bump GGML_MAX_NAME 64 -> 128 (guarded) to fit ACE-Step GGUF tensor names,
matching acestep.cpp's build override.

Add op unit tests and a VAE decode/roundtrip test harness.
… COL2IM_1D)

Add Metal GPU kernels for the two custom ops landed as CPU-only earlier, so
the ACE-Step Oobleck VAE decode can run on GPU (Metal) instead of CPU:

- kernel_snake_f32: per-channel snake activation y = x + sin^2(a*x) * inv_b
- kernel_col2im_1d_f32: scatter-add col2im for the decoder's 1D transposed
  convs, flat grid-stride so occupancy stays high even when OC is tiny
  (final stereo transpose-conv)

Wire both through the device pipeline cache, the op dispatch/encoder, and
ggml_metal_device_supports_op (contiguous f32 in/out). ~4x faster VAE render
on Apple Silicon (M1 Ultra) with output perceptually identical to the CPU path.
Comment thread tests/vae-common.h
#include <vector>

#include <fcntl.h>
#include <sys/mman.h>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This header (and the vae-decode / vae-roundtrip targets that include it) pulls in <sys/mman.h>, <unistd.h>, and <fcntl.h> and calls open/mmap/munmap directly. These are POSIX-only and do not exist on MSVC/Windows, so any test build on Windows (add_executable is unconditional under the existing if (NOT GGML_BACKEND_DL) block) will fail to compile. Since ggml-speech is a cross-platform vcpkg port, either guard these tools behind a if (NOT WIN32) in tests/CMakeLists.txt, or replace the mmap loader with ggml's portable file I/O so the tools build everywhere.

Comment thread include/ggml.h
// ACE-Step DiT tensor names exceed the historical 64-char limit (e.g. 67 chars);
// upstream acestep.cpp bumps this to 128. Keep it as the fork default so the
// ggml lib and every consumer (name[] is part of the ggml_tensor ABI) agree.
# define GGML_MAX_NAME 128

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bumping the default GGML_MAX_NAME from 64 to 128 changes the size and field offsets of struct ggml_tensor (the name[] array is inline in the struct and part of the ABI). Because this lives inside #ifndef GGML_MAX_NAME, any consumer or prebuilt artifact that was compiled against the 64-byte layout and links against a lib built with 128 will silently disagree on struct layout (ODR/ABI mismatch, memory corruption). Confirm every downstream of the ggml-speech port (audiogen-cpp and any other consumer) is rebuilt from this header in the same build, and consider documenting this as a hard rebuild requirement in the changelog/port notes.

Comment thread include/ggml.h
// out = x*aw + ab + max(x,0) + slope*min(x,0).
GGML_OP_AFFINE_PRELU,

// col2im for 1D transpose-conv (ACE-Step Oobleck VAE, QVAC-21921):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The ticket reference QVAC-21921 is embedded in code comments across the diff (public header include/ggml.h at lines ~229-231 and ~610/2508, plus src/ggml.c, src/ggml-cpu/ops.cpp, all four tests/*.cpp and vae-common.h). The team standard forbids adding ticket references anywhere in code, including comments — and it is especially undesirable in a shipped public API header. Remove the QVAC-21921 mentions (keep the descriptive text where useful); track the ticket linkage in the PR/commit metadata instead.

Comment thread src/ggml-cpu/ops.cpp
for (int64_t t_in = t_in_min; t_in <= t_in_max; t_in++) {
const int64_t k = t_abs - t_in * s0;
if (k >= 0 && k < K) {
// col layout [K*OC, T_in]: element (oc*K + k, t_in)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A few added comments narrate what the code does (layout/index restatements here and the mirrored one in ggml-metal.metal) rather than a non-obvious "why". The standard prefers self-explaining code; the index expression already states the layout, so these can be dropped. The math-rationale comments (grid-stride occupancy, the ABI note) are fine to keep — this is only about the "what" restatements.

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.

2 participants