Skip to content

Support dynamic-shifting schedulers in SD3 ControlNet pipelines - #14675

Open
Devam0311 wants to merge 1 commit into
huggingface:mainfrom
Devam0311:sd3-controlnet-dynamic-shifting
Open

Support dynamic-shifting schedulers in SD3 ControlNet pipelines#14675
Devam0311 wants to merge 1 commit into
huggingface:mainfrom
Devam0311:sd3-controlnet-dynamic-shifting

Conversation

@Devam0311

@Devam0311 Devam0311 commented Sep 1, 2026

Copy link
Copy Markdown

Issue link: this addresses Issue 2 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 2 of #13611.

Both SD3 ControlNet pipelines call retrieve_timesteps() with no mu handling and expose no mu argument, so any scheduler with use_dynamic_shifting=True — the SD3.5-style configs — fails before inference:

from diffusers import FlowMatchEulerDiscreteScheduler
from diffusers.pipelines.controlnet_sd3.pipeline_stable_diffusion_3_controlnet import retrieve_timesteps

retrieve_timesteps(FlowMatchEulerDiscreteScheduler(use_dynamic_shifting=True), num_inference_steps=2, device="cpu")
# ValueError: `mu` must be passed when `use_dynamic_shifting` is set to be `True`

The base StableDiffusion3Pipeline and StableDiffusion3InpaintPipeline already compute and pass mu, so ControlNet was the odd one out in the SD3 family.

Solution

Ports the calculate_shift() helper, the mu argument, and the scheduler_kwargs["mu"] handling from the base SD3 pipelines into both ControlNet pipelines.

One ordering change worth calling out. mu is derived from the latents' shape, but these pipelines prepared timesteps (step 4) before latents (step 5) — the reverse of the base SD3 pipelines. So latent preparation now runs first, and the steps are renumbered to match.

That reorder is safe here: prepare_latents only draws from the generator and never touches the scheduler, and retrieve_timesteps never touches the latents. The strongest evidence is that every existing expected slice in tests/pipelines/controlnet_sd3/ still passes unchanged — I ran the suite after the reorder and before adding any new tests.

The alternative was deriving image_seq_len from height / width to avoid moving anything, but that re-derives what prepare_latents already computed and would get the user-supplied-latents case wrong.

Testing

Two tests per pipeline (four total):

  • test_dynamic_shifting_scheduleruse_dynamic_shifting=True now runs, exercising the derived-mu path
  • test_dynamic_shifting_scheduler_accepts_explicit_mu — covers the elif mu is not None branch

Verified all four catch the bug: with the fix reverted, all four fail with the ValueError above.

pytest tests/pipelines/controlnet_sd3/         -> 43 passed, 30 skipped
ruff check / ruff format --check               -> clean
python utils/check_forward_call_docstrings.py  -> all arguments documented
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 step reorder is the part to scrutinise, and it is the only reason this PR is not a pure addition. Reasoning and evidence above; happy to switch to deriving image_seq_len from height/width instead if you'd rather nothing moved.
  2. I used latent_height / latent_width rather than shadowing height / width as pipeline_stable_diffusion_3.py does, since those names are still live later in these pipelines.
  3. I kept the base SD3 pipelines' max_shift default of 1.16 for consistency with them, even though calculate_shift's own signature defaults to 1.15. That discrepancy already exists in pipeline_stable_diffusion_3.py; I did not want to silently change it here.
  4. Scope kept to Issue 2. Issues 1, 3, 4 and 5 of stable_diffusion_3 model/pipeline review #13611 are in Apply the VAE shift factor when decoding in SD3 inpaint #14671, Fix controlnet_pooled_projections tensor handling in SD3 ControlNet pipelines #14673, Do not mutate the source transformer config in SD3ControlNetModel.from_transformer #14672 and Accept tensor ip_adapter_image_embeds in SD3 ControlNet inpainting #14674.

Who can review?

@yiyixuxu @asomoza @hlky

Both SD3 ControlNet pipelines called `retrieve_timesteps()` without any `mu`
handling and exposed no `mu` argument, so any scheduler configured with
`use_dynamic_shifting=True` — the SD3.5-style configs — raised "`mu` must be
passed when `use_dynamic_shifting` is set to be `True`" before inference.
This made ControlNet inconsistent with the rest of the SD3 family.

Port the `calculate_shift()` helper, the `mu` argument and the
`scheduler_kwargs["mu"]` handling from the base SD3 pipelines.

`mu` is derived from the latents, so latent preparation now runs before
timestep preparation, matching the base SD3 ordering. Neither step depends
on the other, and the existing expected slices are unchanged, confirming
the reorder preserves behaviour.

Add regression tests for the derived and explicitly passed `mu` paths in
both pipelines.

Ref huggingface#13611 (Issue 2)

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/M PR with diff < 200 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant