Skip to content
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

Jump to bottom

Fixing implementation of ControlNet-XS #6772

Merged
merged 92 commits into from
Apr 16, 2024
Merged

Fixing implementation of ControlNet-XS #6772

merged 92 commits into from
Apr 16, 2024

Conversation

UmerHA
Copy link
Contributor

@UmerHA UmerHA commented Jan 30, 2024

What does this PR do?

Implements ControlNet-XS with the feedback from PR #5827 incorporated.

See original PR for details.

Who can review?

Core library:

@UmerHA
CheckIn - created DownSubBlocks
2426799
@UmerHA
Added extra channels, implemented subblock fwd
1c36d0b
@UmerHA
Fixed connection sizes
9626ce3
@UmerHA
checkin
b45de06
@UmerHA
Removed iter, next in forward
eb9c59e
@UmerHA
Models for SD21 & SDXL run through
32bf5a7
@UmerHA
Added back pipelines, cleared up connections
b58572b
@UmerHA
Cleaned up connection creation
2b73d02
@UmerHA
added debug logs
93f9d7d
@UmerHA
updated logs
36bb8f9
@UmerHA
logs: added input loading
0878d37
@UmerHA
Update umer_debug_logger.py
72b29de
@UmerHA
log: Loading hint
773721f
@UmerHA
Update umer_debug_logger.py
c6c831a
@UmerHA
added logs
d20d4bf
@UmerHA
Changed debug logging
8145d36
@UmerHA
debug: added more logs
d657233
@UmerHA
Fixed num_norm_groups
1aa848c
@UmerHA
Debug: Logging all of SDXL input
f2e6366
@UmerHA
Update umer_debug_logger.py
507abfa
@UmerHA
debug: updated logs
5641711
@UmerHA
checkim
4039338
@UmerHA
Readded tests
d7c8d43
@UmerHA
Removed debug logs
1d8d5b8
@UmerHA
Fixed Slow Tests
55b7350
@UmerHA
Added value ckecks | Updated model_cpu_offload_seq
6214da7
@UmerHA
accelerate-offloading works ; fast tests work
54bb91d
@UmerHA
Made unet & addon explicit in controlnet
a709ef8
@UmerHA
Updated slow tests
eecf9a5
@UmerHA
Added dtype/device to ControlNetXS
15affe3
@DN6
Copy link
Collaborator

DN6 commented Apr 8, 2024

Looking good overall 👍🏽 Left some comments.

Regarding the naming, we could perhaps call the module ControlNetXSAdapter? Since its behaviour is similar to our other adapters? cc: @yiyixuxu

@UmerHA
Merge branch 'main' into controlnetxs-2
4965d49
@UmerHA
Integrated review feedback
0de4b44 
- Importing ControlNetConditioningEmbedding now
- get_down/mid/up_block_addon now outside class
- renamed `do_control` to `apply_control`
@UmerHA
Merge branch 'controlnetxs-2' of https://github.com/UmerHA/diffusers …
52e7fa8 
…into controlnetxs-2
@UmerHA
Reduced size of test tensors
1d8cad8 
For this, added `norm_num_groups` as parameter everywhere
@yiyixuxu
Copy link
Collaborator

yiyixuxu commented Apr 8, 2024

Regarding the naming, we could perhaps call the module ControlNetXSAdapter? Since its behaviour is similar to our other adapters?

ok!

@UmerHA
Renamed cnxs-Addon to cnxs-Adapter
f334238 
- `ControlNetXSAddon` -> `ControlNetXSAdapter`
- `ControlNetXSAddonDownBlockComponents` -> `DownBlockControlNetXSAdapter`, and similarly for mid/up
- `get_mid_block_addon` -> `get_mid_block_adapter`, and similarly for mid/up
@UmerHA
Copy link
Contributor Author

UmerHA commented Apr 9, 2024

Regarding the naming, we could perhaps call the module ControlNetXSAdapter? Since its behaviour is similar to our other adapters?

ok!

Sounds good. Have renamed

  • ControlNetXSAddon -> ControlNetXSAdapter
  • ControlNetXSAddonDownBlockComponents -> DownBlockControlNetXSAdapter, and similarly for mid/up blocks
  • get_down_block_addon -> get_down_block_adapter, and similarly for mid/up

@yiyixuxu
Copy link
Collaborator

@UmerHA the failing tests look relevant here - can you look into and fix?

@UmerHA
Copy link
Contributor Author

UmerHA commented Apr 11, 2024

@yiyixuxu you were right - should be fixed now. can you rerun the tests?

@UmerHA
Copy link
Contributor Author

UmerHA commented Apr 15, 2024

The only failing test seems very unrelated to this PR:

CleanShot 2024-04-15 at 08 43 29@2x

cc @yiyixuxu

yiyixuxu
yiyixuxu approved these changes Apr 15, 2024
View reviewed changes
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!

@UmerHA
Copy link
Contributor Author

UmerHA commented Apr 15, 2024

@DN6 some of your comments aren't marked as resolved. do you have any more questions/feedback, or is it fine as is?

@DN6 DN6 merged commit fda1531 into huggingface:main Apr 16, 2024
15 checks passed
@UmerHA UmerHA deleted the controlnetxs-2 branch April 16, 2024 16:27
@DN6
Copy link
Collaborator

DN6 commented Apr 16, 2024

@UmerHA All good. We can break up the tests in a follow up PR.

@DN6
Copy link
Collaborator

DN6 commented Apr 17, 2024

@UmerHA We have failing slow tests because the checkpoint doesn't seem to be updated. Can you take care of this please
Raised an issue: #7694

@UmerHA
Copy link
Contributor Author

UmerHA commented Apr 17, 2024

Have updated the models & ran the slow test on a T4 - works now.

XSE42 added a commit to XSE42/diffusers3d that referenced this pull request May 2, 2024
@XSE42
[Sync] diffusers commit fda1531
7d8c5d6 
diffusers commit fda1531
    Fixing implementation of ControlNet-XS
    (huggingface/diffusers#6772)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rardz rardz rardz left review comments

@DN6 DN6 DN6 left review comments

@Luciennnnnnn Luciennnnnnn Luciennnnnnn left review comments

@yiyixuxu yiyixuxu yiyixuxu approved these changes

Assignees

@yiyixuxu yiyixuxu

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@UmerHA @patrickvonplaten @HuggingFaceDocBuilderDev @yiyixuxu @DN6 @rardz @Luciennnnnnn