Skip to content

Fix controlnet_pooled_projections tensor handling in SD3 ControlNet pipelines - #14673

Closed
Devam0311 wants to merge 1 commit into
huggingface:mainfrom
Devam0311:sd3-controlnet-pooled-projections
Closed

Devam0311 wants to merge 1 commit into
huggingface:mainfrom
Devam0311:sd3-controlnet-pooled-projections

Conversation

@Devam0311

@Devam0311 Devam0311 commented Sep 1, 2026 •

Copy link
Copy Markdown

Issue link: this addresses Issue 3 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 3 of #13611, which is also reported independently as #9686.

Both SD3 ControlNet pipelines resolve the public controlnet_pooled_projections argument with:

controlnet_pooled_projections = controlnet_pooled_projections or pooled_prompt_embeds

or evaluates the truthiness of the left operand, so passing any real pooled projection raises:

RuntimeError: Boolean value of Tensor with more than one value is ambiguous

The documented argument is therefore unusable on that path.

Solution

pipeline_stable_diffusion_3_controlnet.py — replace the or with an explicit None check:

if controlnet_config.force_zeros_for_pooled_projection:
    controlnet_pooled_projections = torch.zeros_like(pooled_prompt_embeds)
elif controlnet_pooled_projections is None:
    controlnet_pooled_projections = pooled_prompt_embeds

pipeline_stable_diffusion_3_controlnet_inpainting.py — here the else branch is guarded by if controlnet_pooled_projections is None, so it is only reachable when the value is not None. It could therefore only ever re-assign the value to itself or raise. Removed.

Testing

test_controlnet_pooled_projections_accepts_tensor builds a ControlNet with force_zeros_for_pooled_projection=False — the configuration that reaches the broken branch — and passes a tensor.

Verified it catches the bug: with the fix reverted it fails with the RuntimeError above; with it applied it passes.

pytest tests/pipelines/controlnet_sd3/   -> 40 passed, 30 skipped
ruff check / ruff format --check         -> clean
git diff --check                         -> clean

Before submitting

Self-review notes

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

For the reviewer:

  1. The two pipelines disagree on semantics, and I deliberately did not reconcile that. The text-to-image pipeline consults controlnet_config.force_zeros_for_pooled_projection; the inpainting pipeline instead zeroes whenever the argument is None and never consults that flag. The issue's suggested fix would apply the text-to-image logic to both, but that is a behaviour change beyond fixing the crash, so I kept each pipeline's existing semantics. Happy to align them in a follow-up if you want that.
  2. Scope kept to Issue 3. Issue 1 is in Apply the VAE shift factor when decoding in SD3 inpaint #14671 and Issue 4 in Do not mutate the source transformer config in SD3ControlNetModel.from_transformer #14672; the rest of stable_diffusion_3 model/pipeline review #13611 is untouched.

Who can review?

@yiyixuxu @asomoza @hlky

…ipelines

Both SD3 ControlNet pipelines resolved the public
`controlnet_pooled_projections` argument with
`controlnet_pooled_projections or pooled_prompt_embeds`. `or` evaluates the
truthiness of the left operand, so passing any real pooled projection raises
`RuntimeError: Boolean value of Tensor with more than one value is
ambiguous`, making the argument unusable.

In the text-to-image pipeline, replace the `or` with an explicit `None`
check. In the inpainting pipeline the `else` branch is only reachable when
the value is not `None`, so it could only ever re-assign the value to itself
or raise; drop it.

Add a regression test that passes a tensor with a ControlNet configured with
`force_zeros_for_pooled_projection=False`, which is the path that raised.

Ref huggingface#13611 (Issue 3), huggingface#9686

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.

@github-actions

Copy link
Copy Markdown
Contributor

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. Fixes #1234) or ask a maintainer to add the no-issue-needed label, and it can be reopened.

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!

@github-actions github-actions Bot closed this Sep 13, 2026
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