Run cleanup and honor return_dict for SDXL latent output - #14680
Run cleanup and honor return_dict for SDXL latent output#14680Devam0311 wants to merge 1 commit into
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.
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