Repository navigation
MOSS-Image: extra sample args to window or disable the block-22 negative-branch skip - #40
Conversation
…ive-branch skip moss_uncond_skip=false turns the built-in uncond skip off; moss_uncond_skip_start / moss_uncond_skip_end bound it in sigma-percent of the shifted flow schedule (ComfyUI percent_to_sigma, inclusive). Without these keys every step skips block 22 as before. Logs the number of skipped negative passes per sampling run.
![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: 572baeeb78
ℹ️ 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".
| } else if (version == VERSION_MOSS_IMAGE && moss_uncond_skip.enabled && | ||
| (!moss_uncond_skip.windowed() || | ||
| (sigma >= moss_skip_sigma_low && sigma <= moss_skip_sigma_high))) { |
There was a problem hiding this comment.
When MOSS sampling uses EasyCache or CacheDIT and a cache hit coincides with entering or leaving the new window, the negative prediction can still come from the previous skip configuration. run_condition() calls step_cache.before_condition() before executing the model, and these caches key entries only by the condition pointer, so changing uncond_skip_layers does not invalidate the cached &uncond result. Consequently, a prediction computed without block 22 can continue to be reused outside the requested window, or vice versa, altering CFG results. Track changes to the effective skip configuration and invalidate the negative-condition cache before its next lookup.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, this is real. The step caches key the cached prediction on the condition pointer alone, so at the window edge a reused negative pass could carry the result from the other skip set. 5c4248e tracks the effective negative-branch skip per pass and drops the cached uncond entry whenever it flips, so the first pass after the edge is recomputed. Verified with EasyCache on MOSS Base at 25 steps: steps 3 to 13 reuse the cached uncond, step 14 recomputes it even though EasyCache reuses that step, and the default path without the new keys is still pixel-identical to the current prebuilt. Studio never enables an sd.cpp step cache, so this only matters when someone passes --cache-mode by hand.
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. |
…p set changes EasyCache / UCache / CacheDIT key a condition's cached output by its pointer only, so at the moss_uncond_skip window edge a reused &uncond diff could come from the other skip configuration. Track the effective negative-branch skip per pass and invalidate &uncond in the step cache when it flips. Only runs with a window set; the default path is unchanged. Verbose logs say whether each windowed MOSS uncond pass was computed or served from cache.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5c4248e21b
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
…ch skip (unslothai#16255) ## Summary unslothai#16127 made the diffusers MOSS-Image path skip transformer block 22 on the negative pass per checkpoint, as the reference workflows do: never for MOSS-Image-SFT, and only while sigma-percent is in [0, 0.5] for MOSS-Image-Base. The GGUF path runs stable-diffusion.cpp, which still skips block 22 on every step for both checkpoints. unslothai/stable-diffusion.cpp#40 added sample args that express the per-checkpoint rule. This sends them from Studio and pins the prebuilt that reads them. The pinned release, `master-982-0518b34-uaba3a1c`, is published on unslothai/stable-diffusion.cpp; the 7 asset names `test_sd_cpp_vulkan_cpu_torch.py` lists are all in it. ## Change - `sd_cpp_args.moss_uncond_skip_sample_args`: `moss_uncond_skip=false` for SFT, `moss_uncond_skip_start=0,moss_uncond_skip_end=0.5` for Base. Base is detected from the repo id, folder or GGUF filename (separators folded). `UNSLOTH_MOSS_REFERENCE_SKIP=0` sends nothing, which is the binary's every-step default. - `sd_cpp_backend`: MOSS-Image requests carry it, as `sample_params.extra_sample_args` on the server path and `--extra-sample-args` on the CLI path. Other families send nothing. - `install_sd_cpp_prebuilt.DEFAULT_TAG`: `master-982-0518b34-ud331c66` -> `master-982-0518b34-uaba3a1c`. Same upstream base (master-982); the new build adds only unslothai#40. The tests that name the pin are updated. A build without the keys ignores them, so an older install keeps today's behaviour until it updates. ## Results `MOSS-Image-Base-Q4_K_M.gguf`, Linux x86_64 CPU, 256x256, 25 steps, `euler`, the release's own `sd-cli`: | case | negative passes that skip block 22 | |---|---| | new pin, no keys | 25 of 25; PNG byte-identical to the current pin | | new pin, SFT (`moss_uncond_skip=false`) | 0 of 25 | | new pin, Base (window `0..0.5`) | 13 of 25 (steps 1-13) | | current pin given the keys | keys ignored; identical to the same binary without them | The server path, driven through this branch's `build_img_gen_request` and `SdCppServer`, gives the same counts: 25, 0 and 13 of 25. ## Tests - `test_sd_cpp_moss_uncond_skip.py` (new): Base / SFT detection across repo ids, folders and GGUF names, the env kill switch, the server request field, the CLI flag, and that other families send nothing. - `test_sd_cpp_moss_uncond_skip.py`, `test_sd_cpp_args.py`, `test_sd_cpp_install_verify.py`, `test_sd_cpp_vulkan_cpu_torch.py`, `test_diffusion_more_families.py`: 310 passed. ## Not proven - Only the Linux x86_64 CPU bundle was run. The CUDA, Vulkan, Metal, Windows and aarch64 bundles were built by the release job but not rendered. - The MOSS-Image-SFT GGUF was not rendered on the GGUF path; its 0-of-25 count comes from the Base GGUF given the SFT keys.
Based on
master(d331c66, #34). One source change, no ggml patch.Problem
MOSS-Image always drops transformer block 22 from the negative (unconditional) pass, on every step (
moss_skip_layers = {22}forVERSION_MOSS_IMAGEinsrc/pipeline/diffusion_engine.cpp, from upstream's MOSS-Image support). The reference workflows for these checkpoints do it per checkpoint instead:The existing skip-layer guidance options cannot express either:
--slg-scale > 0adds a third, skipped positive pass, andslg_uncondonly skips inside its window while the MOSS code still forces block 22 outside it.Fix
Three new
--extra-sample-argskeys, read insrc/runtime/guidance.{h,cpp}and applied where the MOSS negative pass picks its skip layers:moss_uncond_skiptruefalseruns the negative pass with no skipped blockmoss_uncond_skip_start0moss_uncond_skip_end1The window uses the schedule's own shift (
--flow-shiftis respected). With none of the keys set, the old code path runs unchanged. Invalid values are warned about and ignored. At each window edge the step caches (EasyCache / UCache / CacheDIT) drop their cached negative prediction and Spectrum clears its history, since both would otherwise reuse outputs computed with the other skip set. Each sampling run logsMOSS uncond skip: K of N steps, and-vlogs one line per skipped step.Studio will send
moss_uncond_skip=falsefor SFT andmoss_uncond_skip_start=0,moss_uncond_skip_end=0.5for Base, matching the diffusers path in unslothai/unsloth#16127.Results
MOSS-Image-Base-Q4_K_M.gguf, Linux x86_64 CPU, 256x256, 25 steps,euler. Builds use the same flags as the release job; pixels compared after decoding (the PNG metadata differs only in the build name).d331c66rebuilt vs the pinned releasemaster-982-0518b34-ud331c660..1moss_uncond_skip=false0..0.50..0.50..0.5The default, SFT and Base renders all differ from each other, so the new path runs.
Not proven
eulerwas checked. On samplers that call the model more than once per step, the summary counts model calls.