Apply the VAE shift factor when decoding in SD3 inpaint - #14671
Open
Devam0311 wants to merge 2 commits into
Open
Apply the VAE shift factor when decoding in SD3 inpaint#14671Devam0311 wants to merge 2 commits into
Devam0311 wants to merge 2 commits into
Conversation
`StableDiffusion3InpaintPipeline` encodes VAE latents with `(latents - shift_factor) * scaling_factor` but decoded with only `latents / scaling_factor`, dropping `+ shift_factor`. Encode and decode therefore disagreed within the same pipeline, and the output differed from `StableDiffusion3Pipeline`, `StableDiffusion3Img2ImgPipeline` and both SD3 ControlNet pipelines, which all apply the shift. SD3 VAEs have a nonzero `shift_factor`, so inpaint output was decoded from the wrong latent distribution. Add a regression test asserting the decoded image matches a manual decode with the shift applied, and update the `test_inference` expected slice, which pinned the previous incorrect output. Ref huggingface#13611 (Issue 1) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XBYeq5vB4DNZDEaqUsroKR
This was referenced Sep 1, 2026
Contributor
|
Hi @Devam0311, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice. Once the PR links an issue (or gets the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Fixes Issue 1 of #13611 (
stable_diffusion_3model/pipeline review).StableDiffusion3InpaintPipelineencodes VAE latents with(latents - shift_factor) * scaling_factor(_encode_vae_image, and the masked-image path), but decoded with onlylatents / scaling_factor— dropping+ shift_factor.So encode and decode disagreed within the same pipeline, and the result differed from every other SD3 pipeline:
pipeline_stable_diffusion_3.py:1136(latents / scaling_factor) + shift_factorpipeline_stable_diffusion_3_img2img.py:1162(latents / scaling_factor) + shift_factorcontrolnet_sd3/..._controlnet.py:1243(latents / scaling_factor) + shift_factorcontrolnet_sd3/..._controlnet_inpainting.py:1402(latents / scaling_factor) + shift_factorpipeline_stable_diffusion_3_inpaint.py:1368latents / scaling_factorSD3 VAEs have a nonzero
shift_factor, so inpaint output was decoded from the wrong latent distribution.Solution
Applies the shift before decoding, matching the sibling pipelines:
Testing
Reproduced first, with the
shift_factor=0.25VAE from the issue:Added
test_vae_shift_factor_applied_on_decode, which decodes the pipeline's own latents both with and without the shift and asserts the pipeline output matches the former. It also asserts the two differ, so the test fails loudly if the dummy VAE'sshift_factorever becomes zero and the test stops being meaningful.Verified the test actually catches the bug: with the one-line fix reverted it fails, with it applied it passes.
test_inference'sexpected_sliceis updated in the same commit — it pinned the previous, incorrect output. The new values were regenerated through the test's own code path (13 of 16 entries moved, max delta ~2e-2, consistent with the dummy VAE'sshift_factor=0.0609)._encode_vae_imageis the only method other pipelines# Copied fromhere, and it is untouched, so copy consistency is unaffected.Before submitting
self-reviewskill on the diff?Self-review notes
Reviewed against
.ai/references/review-rules.md,pipelines.mdandtesting.md. No blocking issues.For the reviewer:
expected_slicechange is the part worth your attention. Changing a pinned slice can hide a regression, so: the delta is confined to the decode path, matches the magnitudeshift_factor=0.0609implies, and the new dedicated test pins the behaviour rather than a magic constant.Not done: no slow/integration test against a real SD3 checkpoint (per
testing.md, those come later). Verified on CPU with tiny dummy components only.Who can review?
@yiyixuxu @asomoza @hlky