-
Couldn't load subscription status.
- Fork 6.5k
Improve control net block index for sd3 #9758
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
yiyixuxu
merged 16 commits into
huggingface:main
from
linjiapro:allow_controlnet_layers
Nov 20, 2024
Merged
Changes from 15 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
a0d199a
improve control net index
linjiapro 48b4b62
wip
linjiapro d1a1ebe
wip
linjiapro bd32b2b
wip
linjiapro a7ffec7
wip
linjiapro 235b800
wip
linjiapro 50b4db9
wip
linjiapro 933ecf3
add rms_norm to controlnet test
linjiapro 9d33417
wip
linjiapro cd3069c
wip
linjiapro e40bd61
wip
linjiapro 4c1d293
merge
linjiapro d8006c3
format
linjiapro b4983cb
wip
linjiapro 5f560ca
Update src/diffusers/models/controlnets/controlnet_sd3.py
linjiapro 570558b
style
yiyixuxu File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
why are we making this change? it is not the same so a breaking change, no?
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.
@yiyixuxu Good question.
The revised code adapts the strategy used by ControlNet for Flux, introducing a significant improvement in flexibility. Here's why this change matters:
In the old code, the number of transformer layers is divisible by the number of ControlNet layers. For example, with SD3.5 Large, which has 38 transformer layers, there were only two valid options for the number of ControlNet layers: 2 and 19. Setting the number of ControlNet layers to anything else, such as 5, would cause the old code to crash.
However, the Flux ControlNet approach removes this restriction, allowing greater flexibility in choosing the number of layers. The revised logic essentially mirrors the Flux implementation, enabling more versatile configurations.
Importantly, the new code maintains compatibility with existing setups. If the number of transformer layers is divisible by the number of ControlNet layers, the interval_control remains unchanged, ensuring all previous configurations continue to function seamlessly.
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.
thanks! I think it's indeed better, I'm just wondering if it would cause issue for controlnet is trained with the current logic
cc @haofanwang here
Uh oh!
There was an error while loading. Please reload this page.
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 don't think it will cause any issue with the trained controlnet using old code before this PR.
The reason is that for the controlnet to be trained with the old code, the number of layers of the transformer has to be divisible by the number of layers of the controlnet, and the new logic after this PR does not change the behavior for the above scenario.