Repository navigation
Conversation
For `output_type="latent"` the SDXL inpaint and instruct-pix2pix pipelines returned `StableDiffusionXLPipelineOutput(images=latents)` immediately. That skipped `maybe_free_model_hooks()`, so model offload cleanup never ran, and bypassed the `return_dict` handling, so `return_dict=False` returned the wrong type. Assign `image = latents` instead and guard watermarking, postprocessing and the padding-mask overlay behind `output_type != "latent"`, so the shared cleanup and return handling at the end runs for every output type — matching the text-to-image pipeline. Applied to the three ControlNet and PAG inpaint pipelines carrying the same early return. Add a regression test asserting cleanup runs and `return_dict=False` returns a tuple of latents. Ref huggingface#13610 (Issue 3) 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 |
adi-IL
left a comment
There was a problem hiding this comment.
Tested locally on Linux x86_64.
Bypassing maybe_free_model_hooks() on output_type='latent' leaked device allocations during CPU/model offloading, and returning StableDiffusionXLPipelineOutput early broke return_dict=False. Setting image = latents and guarding watermarking/postprocessing lets execution reach hook cleanup and return_dict handling across the affected SDXL pipelines. The regression test in test_stable_diffusion_xl_inpaint.py verifies both behaviors cleanly.
|
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 3 of #13610 (
stable_diffusion_xlmodel/pipeline review).For
output_type="latent", the SDXL inpaint and instruct-pix2pix pipelines return early:That return jumps over everything after it, so two things break:
maybe_free_model_hooks()never runs — model offload cleanup is skipped entirely for latent outputreturn_dict=Falseis ignored — the caller gets aStableDiffusionXLPipelineOutputwhere a tuple was requestedReproduced on
main:Solution
Assign
image = latentsrather than returning, and guard watermarking, postprocessing, and the padding-mask overlay behindoutput_type != "latent"— so the shared cleanup andreturn_dicthandling at the end of__call__runs for every output type. This is the shape the text-to-image pipeline already uses.The issue names two files. I swept for the same early return and found three more, all fixed together:
pipeline_stable_diffusion_xl_inpaint.py,pipeline_stable_diffusion_xl_instruct_pix2pix.pypipeline_controlnet_inpaint_sd_xl.py,pipeline_controlnet_union_inpaint_sd_xl.py,pipeline_pag_sd_xl_inpaint.pyThe inpaint pipelines also apply
padding_mask_cropoverlays, which operate on decoded PIL images, so that call moved inside the same guard.Testing
test_latent_output_runs_cleanup_and_honors_return_dictstubsmaybe_free_model_hooksto record whether it fired, requestsoutput_type="latent"withreturn_dict=False, and asserts cleanup ran, a tuple came back, and the payload is still a latent tensor (by channel count —postprocesswould have collapsed it to 3).Verified it catches the bug: with the fix reverted it fails.
The
TestStableDiffusionXL*PipelineIPAdapterclasses are deselected: they fail on a clean checkout ofmainin my environment with aModuleNotFoundErrorfrom an optional dependency I do not have installed, unrelated to this change.Before submitting
self-reviewskill on the diff?Self-review notes
Reviewed against
.ai/references/review-rules.mdandpipelines.md. No blocking issues.For the reviewer:
return_dict=Falsewithoutput_type="latent"now returns a tuple instead of an output object. That is the fix, but it is a return-type change for anyone who was relying on the old behaviour.output_type != "latent"guard, but to disjoint files, so the two do not conflict.padding_mask_cropoverlay moving inside the guard is a deliberate part of the fix, not incidental reindentation — it operates on decoded images and would fail on latents.Who can review?
@yiyixuxu @asomoza @hlky