Repository navigation
Conversation
`StableDiffusion3ControlNetInpaintingPipeline.check_inputs` required `ip_adapter_image_embeds` to be a `list`, but the `__call__` docstring documents a tensor of shape `(batch_size, num_images, emb_dim)` and `prepare_ip_adapter_image_embeds` types it as `torch.Tensor` and calls `.chunk(2)` on it. Passing the documented tensor therefore always failed validation, so precomputed IP-Adapter embeddings could not be used at all. Validate the tensor's rank instead, matching the documented contract. Add regression tests for both the accepted tensor and the rejected rank. Ref huggingface#13611 (Issue 5) 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 5 of #13611.
StableDiffusion3ControlNetInpaintingPipelinecontradicts itself about the type ofip_adapter_image_embeds:__call__docstring (L1111)torch.Tensor… a tensor of shape(batch_size, num_images, emb_dim)"prepare_ip_adapter_image_embeds(L932)torch.Tensor | None, and calls.chunk(2)on itcheck_inputs(L780)listSo passing the documented tensor always failed validation, and precomputed IP-Adapter embeddings could not be used at all. Passing a list to satisfy
check_inputswould then break inprepare_ip_adapter_image_embeds, which calls.chunk(2).The base
StableDiffusion3Pipelinedoes not validate this argument incheck_inputsat all — the list check here appears to be a leftover from the older list-based IP-Adapter API.Solution
Validate the tensor's rank instead of its container type:
Testing
Two tests, driving
check_inputsdirectly:test_check_inputs_accepts_ip_adapter_image_embeds_tensor— the documented 3D tensor now passestest_check_inputs_rejects_ip_adapter_image_embeds_with_bad_ndim— a 2D tensor still raises, with the new messageVerified both catch the bug: with the fix reverted, both fail.
Before submitting
self-reviewskill on the diff?Self-review notes
Reviewed against
.ai/references/review-rules.mdandpipelines.md. No blocking issues.For the reviewer:
listto work around the old check would now fail later insideprepare_ip_adapter_image_embedsrather than early incheck_inputs. I judged that acceptable because the list path was already broken downstream — there is no working list-based caller to preserve — but flagging it since it is technically a behaviour change.ip_adapter_image/ip_adapter_image_embedsmutual-exclusion check above it untouched.Who can review?
@yiyixuxu @asomoza @hlky