Skip to content

Apply the VAE shift factor when decoding in SD3 inpaint - #14671

Open
Devam0311 wants to merge 2 commits into
huggingface:mainfrom
Devam0311:sd3-inpaint-vae-shift
Open

Apply the VAE shift factor when decoding in SD3 inpaint#14671
Devam0311 wants to merge 2 commits into
huggingface:mainfrom
Devam0311:sd3-inpaint-vae-shift

Conversation

@Devam0311

@Devam0311 Devam0311 commented Sep 1, 2026

Copy link
Copy Markdown

Issue link: this addresses Issue 1 of #13611.
I have deliberately not used a Fixes keyword: #13611 tracks 6 separate findings, so a closing keyword would close it as soon as this one merges while the others are still open. Happy for the no-issue-needed label to be applied, or I can add a closing keyword if you would rather the review issue be closed and reopened.

What does this PR do?

Fixes Issue 1 of #13611 (stable_diffusion_3 model/pipeline review).

StableDiffusion3InpaintPipeline encodes VAE latents with (latents - shift_factor) * scaling_factor (_encode_vae_image, and the masked-image path), but decoded with only latents / scaling_factor — dropping + shift_factor.

So encode and decode disagreed within the same pipeline, and the result differed from every other SD3 pipeline:

Pipeline decode
pipeline_stable_diffusion_3.py:1136 (latents / scaling_factor) + shift_factor
pipeline_stable_diffusion_3_img2img.py:1162 (latents / scaling_factor) + shift_factor
controlnet_sd3/..._controlnet.py:1243 (latents / scaling_factor) + shift_factor
controlnet_sd3/..._controlnet_inpainting.py:1402 (latents / scaling_factor) + shift_factor
pipeline_stable_diffusion_3_inpaint.py:1368 latents / scaling_factor

SD3 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:

latents = (latents / self.vae.config.scaling_factor) + self.vae.config.shift_factor
image = self.vae.decode(latents, return_dict=False, generator=generator)[0]

Testing

Reproduced first, with the shift_factor=0.25 VAE from the issue:

max |current - expected|  : 0.675
mean |current - expected| : 0.117

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's shift_factor ever 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's expected_slice is 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's shift_factor=0.0609).

pytest tests/pipelines/stable_diffusion_3/     -> 60 passed, 41 skipped
ruff check / ruff format --check               -> clean
python utils/check_forward_call_docstrings.py  -> all arguments documented
git diff --check                               -> clean

_encode_vae_image is the only method other pipelines # Copied from here, and it is untouched, so copy consistency is unaffected.

Before submitting

Self-review notes

Reviewed against .ai/references/review-rules.md, pipelines.md and testing.md. No blocking issues.

For the reviewer:

  1. The expected_slice change 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 magnitude shift_factor=0.0609 implies, and the new dedicated test pins the behaviour rather than a magic constant.
  2. Scope kept to Issue 1. stable_diffusion_3 model/pipeline review #13611 lists six issues; the other five are untouched and still open.
  3. This changes user-visible SD3 inpaint output. That is the intent — prior output was decoded incorrectly — but it will move any downstream pinned outputs.

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

`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
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
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. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

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 no-issue-needed label), you can ignore this message — it stays here as a comment, but it no longer applies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pipelines size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant