Repository navigation
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
|
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 PR has been automatically closed because it does not link an issue and the reminder above was not addressed within 10 days. If this PR is still relevant, please link the issue it fixes (e.g. We are experimenting with this process to keep the review queue manageable, and it will sometimes get it wrong. If you think this PR should stay open, please just say so here and we will reopen it — no need to justify it at length. Thanks again for contributing, and sorry for the noise if we closed this by mistake! |
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