Repository navigation
Resync with upstream master-1050 and port the ggml patches - #52
danielhanchen wants to merge 72 commits into
Conversation
Co-authored-by: leejet <leejet714@gmail.com>
…2273) Co-authored-by: mirek190 <mirek190@gmail.com>
Conflicts resolved keeping both sides: - build.yml: ours (the mirror's customised upstream CI). - compute_workspace: upstream now always allocates through the scheduler (gallocr path and needs_scheduler removed); the CPU BLAS backend, its thread count, residency accounting and scratch release are carried on top of that. - ggml_extend_backend.cpp: upstream moved sd_backend_dev_buffer_from_host_ptr to ggml_compat.cpp; the CPU weight repack helpers and sd_backend_blas_init stay. - ggml_runner.cpp: upstream's scheduler-only execute_segment (eval callback always set, pre-compute hook) plus the BLAS thread count; runner_end also drops the BLAS scratch. - minimax_h3.hpp: upstream's project_qkv/attend/project_output, INT8-aware MLP scales and chunked native-GLU MLP, with the mirror's head-major fused attention, folded MLP and fused segment block as fast paths in front; the old segment helpers upstream deleted are dropped. - minimax_h3_vae.hpp: upstream's native GLU (leejet#2250) for the decoder feed-forward. - model_manager.cpp: both includes. - diffusion_engine.cpp: P-H3-Image squeeze plus the temporal tiling retry bookkeeping. - cmake/ggml.cmake: upstream now resolves the ggml private include dir only for SD_USE_UPSTREAM_GGML, which left the fused-op probe reading a non-existent ggml.h and silently building MiniMax-H3 unfused; the probe now finds ggml.h from the ggml target.
…submodule) Upstream master-1050 moves the ggml submodule from dc99877 to 135ce1e (57 commits), which moves ROPE_APPLY, SAGE_ATTN, SOL_ATTN and QUANTIZE_I8_CONVROT out of ggml into stable-diffusion.cpp backend extensions. 0002-0004, 0006 and 0007 no longer applied and were ported by cherry-picking each patch onto the new submodule commit: - 0002: upstream 0d84d52a added its own 128x128 wide tile for head size 128 (config row, instance file, shared-memory helper); the port reuses it, keeps only the head size 64 instance, and gives the helper an nstages override for the long-sequence variant. - 0003: context only. - 0004: the fused ops now follow GLU directly in the op enum (GGML_OP_COUNT 104). - 0006: the SAGE_ATTN half (sage-attn.cu hook, GGML_CUDA_CUDNN_SAGE) is dropped with the op; cuDNN now serves FLASH_ATTN_EXT only. - 0007: the rope_apply PDL race fix is dropped with the op; the extension kernel that replaces it launches without PDL. 0010 (Metal ROPE_APPLY kernel) is dropped: GGML_OP_ROPE_APPLY no longer exists in ggml and ggml's extension registry has no Metal hook, so Metal runs the unfused rope graph again. 0001, 0005, 0008, 0009 and 0011-0013 apply unchanged.
Upstream's CUDA build includes cmake/cutlass.cmake, which stops at configure time when thirdparty/cutlass is empty. The source stamp only fetched ggml, so every CUDA leg would fail. Fetch the submodule shallowly and keep only include/ and the license in the tarball.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub retired the macos-14 image, so the Darwin-macOS-arm64 leg is cancelled before it starts
("The configuration 'macos-14-arm64-us-default' is not supported"). macos-15 is Apple Silicon too,
and CMAKE_OSX_DEPLOYMENT_TARGET=14.0 keeps the binaries running on macOS 14.
GitHub Actions
Inactive
GitHub Actions
Inactive
GitHub Actions
Inactive
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f12cd92e3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SD_API bool generate_image(sd_ctx_t* sd_ctx, | ||
| const sd_img_gen_params_t* sd_img_gen_params, | ||
| sd_image_t** images_out, | ||
| int* num_images_out); | ||
| sd_image_generation_result_t** result_out); |
There was a problem hiding this comment.
For applications or language bindings compiled against the previous library, replacing the arguments while retaining the same exported C symbol silently breaks the ABI: generate_image now writes an sd_image_generation_result_t* where callers expect an sd_image_t*, and never updates their image count. Such callers can misinterpret the returned allocation or crash when accessing or freeing it. The same incompatibility affects generate_video, upscale, and adetail_image. Keep the existing entry points as compatibility wrappers and expose metadata-bearing results through new symbols. The repository explicitly requires public-header compatibility unless a breaking change is requested.
AGENTS.md reference: AGENTS.md:L74-L76
Useful? React with 👍 / 👎.
| std::string sd_cpp_extra_args_str = extract_and_remove_sd_cpp_extra_args(request.gen_params.prompt); | ||
| for (auto& bytes : images_bytes) { |
There was a problem hiding this comment.
For /v1/images/edits requests using ordinary SD1/SDXL models, or requests supplying a mask, removing the init-image policy leaves the uploaded image only in ref_images; this builder no longer assigns init_image unless the client supplies a separate prompt-embedded override. There is no replacement fallback in the common request conversion or pipeline: src/pipeline/image.cpp only encodes an initial image when init_image.data is present and otherwise generates a fresh latent. Consequently, previously working image edits ignore the source image, and masked edits cannot preserve its unmasked content. Restore the model- and mask-dependent assignment of the uploaded image to init_image.
Useful? React with 👍 / 👎.
Summary
This merges upstream
master-1050-58bb3c1into the fork and ports the carried ggml patches onto the new ggml submodule commit (135ce1e0). The current Studio pin,master-982-0518b34-uaba3a1c, is 68 upstream commits behind. Among them are leejet#2183 (the model manager reads free memory again before it gives up, also vendored on its own in #50) and leejet#2273 (faster MOSS normalization and staging).Why now
MOSS-Image-SFT Q8_0 with Qwen3.5-9B Q8_0 through
sd-server, 1024x1024, 25 steps. One Colab L4 job, both arms on the same card, with VRAM held down to 7.5 GiB free at load. Each cell ran twice, and repeats agree within 1.7 s.master-982-0518b34-uaba3a1c)7c062d2, patches applied)So this branch is about 1.3x faster on the normal path and about 1.7x faster under memory pressure. Both renders are correct images.
In a separate job, a plain upstream build of the same base (
58bb3c1, no fork patches) next to this branch took 89.5 s vs 101.7 s with nothing taken, and 89.9 s vs 89.5 s at 2 GiB. Sampling took 76.1 s vs 68.2 s and 75.3 s vs 74.7 s, so the ported patches cost MOSS nothing. At 1.5 GiB free, the pin, plain upstream and this branch all fail the same way (the text encoder segment needs 1543 MB, 1534 MB is free). That case is a real shortfall, not something an engine update fixes. unslothai/unsloth#16815 makes Studio report it as out of memory.Changes
64fb9d4: merge ofmaster-1050-58bb3c1. Conflicts were resolved keeping both sides (fork-only server and workflow pieces plus upstream's new code).d4ed8d3: upstream moves ROPE_APPLY, SAGE_ATTN, SOL_ATTN and QUANTIZE_I8_CONVROT out of ggml and into stable-diffusion.cpp backend extensions, so several patches inscripts/unsloth/ggml-patchesno longer applied:7c062d2: the prebuilt workflow now also fetchesthirdparty/cutlass. Upstream's CUDA build includescmake/cutlass.cmake, which stops at configure time without the CUTLASS headers. The source stamp only fetched ggml, so every CUDA leg would have failed. The step keeps onlyinclude/and the license, which keeps the source tarball small.6f12cd9: the macOS arm64 leg moves frommacos-14tomacos-15. GitHub retired the macos-14 image, so that leg was cancelled before it started (The configuration 'macos-14-arm64-us-default' is not supported). master has the same runner label.CMAKE_OSX_DEPLOYMENT_TARGET=14.0stays, so the binaries still run on macOS 14.Testing
135ce1e0. The CPU and CUDA builds of ggml (withtest-backend-ops) and ofsd-serversucceed with the patches applied.6f12cd9(publish=false, https://github.com/unslothai/stable-diffusion.cpp/actions/runs/42178497743): every leg is green, including macOS arm64 onmacos-15. That covers Linux CPU, Vulkan, aarch64, CUDA 12 and CUDA 13; Windows CPU, Vulkan, CUDA 12 and CUDA 13; and macOS arm64 and x86_64. The first CUDA build of the new ggml commit with the CUTLASS kernels is cold and takes 1.3 to 2.8 hours (run 42157974628). With ccache warm, the CUDA legs took 4 to 7 minutes in this run.sd-clirender of MOSS-Image-SFT Q8_0 renders with the patches applied (18.2 s) and without them (66.8 s), image saved both times.Not verified yet
Once it is merged and the nightly publishes, Studio's
DEFAULT_TAGcan move to the new tag, and #50 becomes redundant.