Skip to content

chore: sync ggml and remove obsolete conv1d workarounds - #2147

Merged
leejet merged 1 commit into
masterfrom
chore/sync-ggml
Oct 16, 2026
Merged

chore: sync ggml and remove obsolete conv1d workarounds#2147
leejet merged 1 commit into
masterfrom
chore/sync-ggml

Conversation

@leejet

@leejet leejet commented Oct 16, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Owner

Summary

  • Sync ggml and remove obsolete conv1d workarounds

Related Issue / Discussion

resolve #2109.

Additional Information

N/A

Checklist

Summary by CodeRabbit

  • Improvements
    • Audio encoding now processes waveform streams together, with normalized outputs returned in the expected layout.
    • Convolution processing uses a dedicated full-precision path when requested, with consistent output formatting across batch sizes.

@coderabbitai

coderabbitai Bot commented Oct 16, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 87ebcd8c-6d9b-45fd-abd7-6646d6219a11
📥 Commits

Reviewing files that changed from the base of the PR and between 2f016e8 and 6f43069.

📒 Files selected for processing: 4
  • ggml
  • src/core/ggml_extend.cpp
  • src/model/common/ggml_block.hpp
  • src/model/vae/minimax_h3_audio_vae.hpp

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

📝 Walkthrough

The ggml submodule reference and conv_1d implementation changed. MiniMax H3 audio encoding now processes streams in a single batched pass instead of processing and concatenating each stream separately.

Changes: ggml and Audio VAE Updates

Layer / File(s) Summary
ggml reference and convolution handling
ggml, src/core/ggml_extend.cpp, src/model/common/ggml_block.hpp		 The ggml submodule reference changed. conv_1d uses ggml_conv_1d when F32 accumulation is not forced; the forced-F32 path always reshapes and permutes the result. The include order changed without behavior changes.
Batched MiniMax H3 audio encoding
src/model/vae/minimax_h3_audio_vae.hpp		 AudioVAE::encode now processes the waveform streams together, normalizes the resulting latents, and returns the permuted tensor.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other

Suggested reviewers: noctrex, jk212h20

Merge Risk: ⚪ Minimal · up to 6f430

No confirmed issue currently blocks merging after normal checks.

Security architecture risk: 🔵 Low · up to 6f430

The inspected changes preserve per-clip ownership and existing input checks, without identifying a new security boundary crossing. Compatibility of the batched encoder with the updated backend remains unverified because that backend revision was unavailable for inspection.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The inspected audio path preserves one latent entry per reference clip. Batching therefore does not itself combine data from separate reference clips or establish expanded tenant exposure; lower-level tensor ordering remains unresolved.

Trust Boundaries and Controls

  • observed — The inspected caller retains its existing input-array validation, reference-mode restrictions, waveform validation, and empty-result checks. The full PR comparison does not change those controls or the caller of the audio encoder.

Resilience and Maintainability Implications

  • observed — The runner's single-active-computation guard and cleanup behavior predate this PR. Arbitrary concurrent use is not established by that unsynchronized guard, and no explicit asynchronous cancellation contract was found; these remain existing limitations rather than demonstrated batching regressions.
Pre-merge checks: Passed 3 · Failed 1 · Inconclusive 1

Failed Failed checks: 1 warning · 1 inconclusive

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Linked Issues check Inconclusive Issue #2109 requires ggml 0.26.0. The PR changes the ggml submodule from d25b121ce1d6ae0c7e62ad080460d9fc1e46dd93 to dc998771e700d49bbdf9a39cede22bb5b3311767 and updates related conv1d workarounds. Th… Provide reviewable evidence that ggml revision dc998771e700d49bbdf9a39cede22bb5b3311767 declares version 0.26.0.
Passed Passed checks: 3 passed
Check name Status Explanation
Title check Passed The title clearly summarizes the main changes: syncing ggml and removing obsolete conv1d workarounds.
Description check Passed The description includes all required sections, identifies the changes, links issue #2109, provides additional information, and confirms the contribution guidelines checklist.
Out of Scope Changes check Passed The conv1d changes in src/core/ggml_extend.cpp and the batched MiniMax H3 VAE changes remove workarounds for the updated ggml behavior. The include reorder has no functional effect. These changes supp…
Full details: Out of Scope Changes check

Explanation

The conv1d changes in src/core/ggml_extend.cpp and the batched MiniMax H3 VAE changes remove workarounds for the updated ggml behavior. The include reorder has no functional effect. These changes support the ggml synchronization and do not show unrelated scope.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (1 skipped: 1 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Full details: Linked Issues check

Explanation

Issue #2109 requires ggml 0.26.0. The PR changes the ggml submodule from d25b121ce1d6ae0c7e62ad080460d9fc1e46dd93 to dc998771e700d49bbdf9a39cede22bb5b3311767 and updates related conv1d workarounds. The available repository evidence does not show the version declared by the new submodule revision.

Resolution

Provide reviewable evidence that ggml revision dc998771e700d49bbdf9a39cede22bb5b3311767 declares version 0.26.0.

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@leejet
leejet merged commit af0fc72 into master Oct 16, 2026
11 checks passed
@leejet
leejet deleted the chore/sync-ggml branch October 16, 2026 19:39
@leehack leehack mentioned this pull request Oct 17, 2026
Open
6 tasks
@laurentvv laurentvv mentioned this pull request Oct 17, 2026
Open
Filve pushed a commit to Filve/diffusione-in-tempo-reale that referenced this pull request Oct 18, 2026
@claude
Merge upstream/master (leejet/stable-diffusion.cpp b13a482) into fix_…
2e35471 
…to_claude

Brings in the 20 upstream commits since 2988060 (2026-10-11): per-module
automatic flash attention (leejet#2132), automatic direct convolution (leejet#2134,
leejet#2141), reclaim before trusting wrapped Vulkan iGPU memory estimates
(leejet#2124), UTF-8 file paths on Windows (leejet#2144, leejet#2145), FP8 CPU crash fix
(leejet#2140), the ggml sync to dc99877 (leejet#2147), FLUX.3 Klein, MOSS-Image, the
MoSR upscaler, MiniMax-H3 LoRAs, Qwen Image 2.1 Turbo preset, ROCm and
MUSA Dockerfiles, documentation.

Conflicts resolved, keeping both sides:

- include/stable-diffusion.h: upstream's flash_attn_mode and
  conv_direct_mode stay right after conditioning_cache_size, so the
  layout up to there is still upstream's; memory_guard,
  disable_backend_fallback and multi_gpu follow.
- src/core/ggml_extend_backend.cpp: upstream's new cuda_compiled_arch()
  keeps asking the compute capability through sd::cuda_driver (the driver
  is looked up at run time, so the engine starts where there is no NVIDIA
  driver), not through <cuda.h>; the HIP include from upstream is kept.
- src/model_manager.cpp: the CPU memory guard and upstream's
  requires_reclamation rule are both applied: a Vulkan iGPU estimate that
  wrapped is reclaimed before it is trusted, and the guard still decides
  for the CPU backend.
- src/model_loader.cpp: includes of both sides; load_worker keeps the
  file handle pool and the open retry.
- examples/server/runtime.cpp: --degradation-cooldown check and
  upstream's u8path for the --serve-html-path check.
- docs/backend.md: the "Several GPUs" section and upstream's "Direct
  convolution" and "Flash attention" sections.

The ggml submodule moves from d25b121 to dc99877 (upstream's pin).

Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01EhZzDBkuuMp1jBf4WtEySC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

See More

Successfully merging this pull request may close these issues.

[Feature] Upgrade ggml backend to v0.26.0

1 participant

@leejet