Skip to content

Fix flux controlnet mode to take into account batch size #9406

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
+17 −14
Closed

Fix flux controlnet mode to take into account batch size #9406

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
370f382
flux controlnet mode to take into account batch size
christopher-beckham Sep 10, 2024
c333d89
incorporate yiyixuxu's suggestions (cleaner logic) as well as clean u…
christopher-beckham Sep 18, 2024
61e0950
Merge branch 'main' into christopher-beckham/fix_flux_controlnet_modes
yiyixuxu Sep 23, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/diffusers/pipelines/flux/pipeline_flux_controlnet.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -748,9 +748,11 @@ def __call__(
)

# set control mode
orig_mode_type = type(control_mode)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we only accept int for single controlnet, maybe raise a value error here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes good idea

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There does already appear to be a check for it here inside the controlnet class itself:

diffusers/src/diffusers/models/controlnet_flux.py

Lines 289 to 290 in b52119a

if controlnet_mode is None:
raise ValueError("`controlnet_mode` cannot be `None` when applying ControlNet-Union")

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should accept both int or a list (of length 1) int

Suggested change
orig_mode_type = type(control_mode)
if isinstance(control_mode, list):
control_mode = [control_mode]
if len(control_mode) > 1:
raise ValueError(" For `FluxControlNet`, `control_mode` should be an `int` or a list contain 1 `int`")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you meant to do the opposite here? i.e. if not isinstance(control_mode, list) then convert control_mode into a singleton list?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops! yes!

if control_mode is not None:
control_mode = torch.tensor(control_mode).to(device, dtype=torch.long)
control_mode = control_mode.reshape([-1, 1])
control_mode = torch.tensor(control_mode).to(device, dtype=torch.long).view(-1,1)
if orig_mode_type == int:
control_mode = control_mode.repeat(control_image.shape[0], 1)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
control_mode = torch.tensor(control_mode).to(device, dtype=torch.long).view(-1,1)
if orig_mode_type == int:
control_mode = control_mode.repeat(control_image.shape[0], 1)
control_mode = torch.tensor(control_mode).to(device, dtype=torch.long)
control_model = control_mode.view(-1,1).expand(control_image.shape[0], 1)


elif isinstance(self.controlnet, FluxMultiControlNetModel):
control_images = []
Expand Down Expand Up @@ -793,8 +795,10 @@ def __call__(
control_mode_.append(-1)
else:
control_mode_.append(cmode)
control_mode = torch.tensor(control_mode_).to(device, dtype=torch.long)
control_mode = control_mode.reshape([-1, 1])
control_mode = torch.tensor(control_mode_).to(device, dtype=torch.long)
control_mode = control_mode.view(-1, 1)
else:
raise ValueError("For multi-controlnet, control_mode should be a list")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to make sure each control_mode has batch_size too for multi-controlnet

Suggested change
control_mode = torch.tensor(control_mode_).to(device, dtype=torch.long)
control_mode = control_mode.view(-1, 1)
else:
raise ValueError("For multi-controlnet, control_mode should be a list")
control_mode = torch.tensor(control_mode_).to(device, dtype=torch.long)
control_mode = control_mode.view(-1, 1).expand(control_images[0].shape[0]
else:
raise ValueError("For multi-controlnet, control_mode should be a list")

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok I made this change as well. (Will commit in a sec)

However, unlike the regular controlnet if block I do not explicitly allow for control_mode to be an int here, i.e. it won't automagically be converted to a singleton list. I think maybe in the "multi" case it's best to be explicit -- and remind the user -- to think in terms of it being a list, even if the multi controlnet object just has one controlnet contained inside it.


# 4. Prepare latent variables
num_channels_latents = self.transformer.config.in_channels // 4
Expand Down