-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
flux controlnet fix (control_modes batch & others) #9507
Conversation
…p control mode handling for multi case
…rolnet_modes' into flux_controlnet_modes_yiyi
control_single_block_samples, single_block_samples | ||
) | ||
] | ||
if block_samples is not None and control_block_samples is not None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flux controlnet output can contain None
depends on the controlnet has single layers or not,
e.g. union has it https://huggingface.co/InstantX/FLUX.1-dev-Controlnet-Union/blob/main/config.json
but canny does not https://huggingface.co/InstantX/FLUX.1-dev-Controlnet-Canny/blob/main/config.json#L16
control_mode = torch.tensor(control_mode).to(device, dtype=torch.long) | ||
control_mode = control_mode.reshape([-1, 1]) | ||
control_mode = control_mode.view(-1, 1).expand(control_image.shape[0], 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see more context on this PR #9406
guidance = ( | ||
torch.tensor([guidance_scale], device=device) if self.controlnet.config.guidance_embeds else None | ||
) | ||
if isinstance(self.controlnet, FluxMultiControlNetModel): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR #9472 makes it possible to use a schnell base with dev controlnet, however it breaks the case when it is a FluxMultiControlNetModel because FluxMultiControlNetModel does not config
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is safer to use this :
in controlnet_flux.py, in the foward method check this:
if isinstance(self.time_text_embed, CombinedTimestepTextProjEmbeddings):
guidance = None
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM I also tested it just in case.
One suggestion I have, it doesn't appear that any checking is actually done to see the lengths of all the arguments in zip are the same length. This could lead to a "silent bug" where e.g. the user may accidentally mis-specify something like diffusers/src/diffusers/models/controlnet_flux.py Lines 479 to 484 in 560449d
It might be useful to add a check in for that before the zip call. I wrote this below: lengths = [
len(controlnet_cond), len(controlnet_mode), len(conditioning_scale),
len(self.nets)
]
if len(set(lengths)) != 1:
raise ValueError(
"`controlnet_cond`, `controlnet_mode`, `conditioning_scale`, `self.nets` " +
f" must all have the same length, we got (respectively): {lengths}"
) Otherwise it lgtm. Thanks! Edit: actually ideally it would also be good to check if they're lists too... Type checking in Python is annoying, isn't it? |
@christopher-beckham nice suggestion! thanks! |
@yiyixuxu thanks! Just fyi I have something already in the works for the input checking, and can PR that today if you want. It's actually a lot easier than I thought it was, just some logic which needs to be pulled in from the corresponding controlnet pipeline for SDXL. Thanks. |
* flux controlnet mode to take into account batch size * incorporate yiyixuxu's suggestions (cleaner logic) as well as clean up control mode handling for multi case * fix * fix use_guidance when controlnet is a multi and does not have config --------- Co-authored-by: Christopher Beckham <[email protected]> Co-authored-by: Sayak Paul <[email protected]>
* flux controlnet mode to take into account batch size * incorporate yiyixuxu's suggestions (cleaner logic) as well as clean up control mode handling for multi case * fix * fix use_guidance when controlnet is a multi and does not have config --------- Co-authored-by: Christopher Beckham <[email protected]> Co-authored-by: Sayak Paul <[email protected]>
this PR:
control_mode
shape whennum_images_per_prompt > 1
(continue the PR from @christopher-beckham Fix flux controlnet mode to take into account batch size #9406)slow tests here