Skip to content

support sd3.5 for controlnet example #9860

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

Merged
merged 19 commits into from
Dec 6, 2024

Conversation

DavyMorgan
Copy link
Contributor

What does this PR do?

Fixes # (issue)

Before submitting

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@DavyMorgan
Copy link
Contributor Author

@sayakpaul Hi, in this PR, I add sd3.5 to the controlnet example, with only a few lines added!

@sayakpaul sayakpaul requested a review from yiyixuxu November 4, 2024 17:06
Copy link
Collaborator

@yiyixuxu yiyixuxu left a comment

Choose a reason for hiding this comment

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

thanks!

@HuggingFaceDocBuilderDev

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.

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Nov 4, 2024

@DavyMorgan can you fix the test?

@DavyMorgan
Copy link
Contributor Author

DavyMorgan commented Nov 4, 2024

@DavyMorgan can you fix the test?

Yeah, I have updated the test.
@yiyixuxu @sayakpaul Could you please run the test workflows again?

@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Nov 8, 2024

@DavyMorgan
would you be able to update the branch to sync with the main (and resolve the conflicts)?
sorry about that!

@DavyMorgan
Copy link
Contributor Author

@DavyMorgan would you be able to update the branch to sync with the main (and resolve the conflicts)? sorry about that!

Sure, I can fix that.

@DavyMorgan
Copy link
Contributor Author

@yiyixuxu @sayakpaul Could you please run the test workflows again?

@DavyMorgan
Copy link
Contributor Author

@sayakpaul I updated the codes. This time it should be ok. :)

Copy link
Member

@sayakpaul sayakpaul left a comment

Choose a reason for hiding this comment

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

Thanks for the changes!

Could you provide some example results as well?

Additionally, I hosted the tiny pipe to our testing org:
https://huggingface.co/hf-internal-testing/tiny-sd35-pipe

Could you please change the path accordingly in test_controlnet.py?

I will let @yiyixuxu review the other changes introduced.

@@ -986,7 +989,7 @@ def main(args):
controlnet = SD3ControlNetModel.from_pretrained(args.controlnet_model_name_or_path)
else:
logger.info("Initializing controlnet weights from transformer")
controlnet = SD3ControlNetModel.from_transformer(transformer)
controlnet = SD3ControlNetModel.from_transformer(transformer, num_extra_conditioning_channels=0)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be hard-coded like this? Can we use a CLI arg?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I have added a CLI arg.

@DavyMorgan
Copy link
Contributor Author

https://huggingface.co/hf-internal-testing/tiny-sd35-pipe

@sayakpaul I have updated the codes to use hf-internal-testing/tiny-sd35-pipe. Meanwhile, the example results can be found at the end of examples/controlnet/README_sd3.md.

@@ -344,7 +343,7 @@ def custom_forward(*inputs):

# controlnet residual
if block_controlnet_hidden_states is not None and block.context_pre_only is False:
interval_control = len(self.transformer_blocks) // len(block_controlnet_hidden_states)
interval_control = int(math.ceil(len(self.transformer_blocks) / len(block_controlnet_hidden_states)))
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry I missed this change - any reason we make this change 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.

@yiyixuxu Yes, this change is very much related to #9758 and also inspired by the flux-controlnet implementation, which avoids crash when transitioning from sd3 to sd3.5.

@yiyixuxu
Copy link
Collaborator

related ( a little bit overlap with) #9758

@DavyMorgan
Copy link
Contributor Author

@yiyixuxu @sayakpaul Can you run the tests again to check whether it works well with the current HEAD?

@sayakpaul
Copy link
Member

@DavyMorgan sorry for the late reply. Could you consider the changes introduced in #9758 and repurpose this PR accordingly?

@@ -56,6 +56,10 @@ def __init__(
pooled_projection_dim: int = 2048,
out_channels: int = 16,
pos_embed_max_size: int = 96,
dual_attention_layers: Tuple[
Copy link
Collaborator

Choose a reason for hiding this comment

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

we already have this change

dual_attention_layers: Tuple[int, ...] = (),

can you rebase?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@yiyixuxu done.

@DavyMorgan
Copy link
Contributor Author

@yiyixuxu @sayakpaul Hi, I have fixed the codes to pass the quality test. Please run the tests again!

@DavyMorgan
Copy link
Contributor Author

DavyMorgan commented Dec 6, 2024

Sorry for missing the typo😂Now fixed. @yiyixuxu @sayakpaul

@yiyixuxu yiyixuxu merged commit 6131a93 into huggingface:main Dec 6, 2024
15 checks passed
sayakpaul pushed a commit that referenced this pull request Dec 23, 2024
* support sd3.5 in controlnet

---------

Co-authored-by: YiYi Xu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants