[Refactor] refactor `loaders.py` to make it cleaner and leaner. by sayakpaul · Pull Request #5771 · huggingface/diffusers

sayakpaul · 2023-11-12T07:32:03Z

What does this PR do?

I had to move some import statements to not cause circular dependencies.

patrickvonplaten · 2023-11-13T13:33:50Z

@@ -0,0 +1,201 @@
+# Copyright 2023 The HuggingFace Team. All rights reserved.


Can we remove this file and put everything in the corresponding classes? I think most things here should go into lora.py

Just kept it because of AttnProcLayers as it is used by custom diffusion.

patrickvonplaten · 2023-11-13T13:34:39Z

+    set_adapter_layers,
+    set_weights_and_activate_adapters,
+)
+from .utils import PatchedLoraProjection, text_encoder_attn_modules, text_encoder_mlp_modules


Let's define all these classes directly here. The classes are only used in lora.py anyways no?

text_encoder_lora_state_dict() is used by some training scripts. I moved it to lora.py as well and made necessary changes. Is that too aggressive?

patrickvonplaten

Looks good to me! Let's make sure we run all slow tests related to text inv, lora and single file and unet2D. Especially the slow tests of:

LoRA
diffusers/tests/pipelines/stable_diffusion/test_stable_diffusion.py

Line 1118 in b1fbef5

class StableDiffusionPipelineCkptTests(unittest.TestCase):
https://github.com/huggingface/diffusers/blob/main/tests/models/test_attention_processor.py
https://github.com/huggingface/diffusers/blob/main/tests/models/test_models_unet_2d_condition.py

patrickvonplaten · 2023-11-13T13:40:59Z

Very cool refactor - let's make sure we don't braek

Looks good to me! Let's make sure we run all slow tests related to text inv, lora and single file and unet2D. Especially the slow tests of:

LoRA

https://github.com/huggingface/diffusers/blob/b1fbef544c993288447181a6c8d8c68d89387ebe/tests/pipelines/stable_diffusion/test_stable_diffusion.py#L1118https://github.com/huggingface/diffusers/blob/main/tests/models/test_attention_processor.py

https://github.com/huggingface/diffusers/blob/main/tests/models/test_models_unet_2d_condition.py

Once all these slow tests run correctly we can merge this IMO

HuggingFaceDocBuilderDev · 2023-11-14T04:37:38Z

The documentation is not available anymore as the PR was closed or merged.

sayakpaul · 2023-11-14T04:44:49Z

@patrickvonplaten I have run all the tests you asked for, and they pass :=)

I'd appreciate it very much if you could go through the PR one more time rigorously and let me know your feedback.

patrickvonplaten · 2023-11-14T11:07:42Z

Added two functions to src/diffusers/loaders/__init__.py to make sure the imports are backwards compatible for imports from example scripts. Apart from this everything looks good to me! Let's try to get this in asap to avoid merge conflicts.

sayakpaul · 2023-11-14T11:14:57Z

Thanks! Will merge once the CI is green then!

…ingface#5771) * refactor loaders.py to make it cleaner and leaner. * refactor loaders init * inits. * textual inversion to the init. * inits. * remove certain modules from the main init. * AttnProcsLayers * fix imports * avoid circular import. * fix circular import pt 2. * address PR comments * imports * fix: imports. * remove from main init for avoiding circular deps. * remove spurious deps. * fix-copies. * fix imports. * more debug * more debug * Apply suggestions from code review * Apply suggestions from code review --------- Co-authored-by: Patrick von Platen <patrick.v.platen@gmail.com>

refactor loaders.py to make it cleaner and leaner.

78a4873

sayakpaul requested review from DN6 and patrickvonplaten November 12, 2023 07:33

sayakpaul added 6 commits November 12, 2023 13:18

refactor loaders init

696b969

inits.

ce5e2ee

textual inversion to the init.

8b04ab2

inits.

a8ef5ca

remove certain modules from the main init.

40c13bc

AttnProcsLayers

c2e75cd

sayakpaul changed the title ~~[WIP][Refactor] refactor loaders.py to make it cleaner and leaner.~~ [WIP][Refactor] refactor loaders.py to make it cleaner and leaner. Nov 13, 2023

sayakpaul added 3 commits November 13, 2023 08:06

fix imports

3fa4db0

avoid circular import.

442c8ec

fix circular import pt 2.

10fbc32

patrickvonplaten reviewed Nov 13, 2023

View reviewed changes

Comment thread examples/text_to_image/train_text_to_image_flax.py Outdated

patrickvonplaten reviewed Nov 13, 2023

View reviewed changes

Comment thread examples/text_to_image/train_text_to_image_lora_sdxl.py Outdated

patrickvonplaten reviewed Nov 13, 2023

View reviewed changes

patrickvonplaten approved these changes Nov 13, 2023

View reviewed changes

Comment thread src/diffusers/loaders.py Outdated

sayakpaul added 2 commits November 13, 2023 20:05

Merge branch 'main' into refactor/loaders

b00f299

address PR comments

ca011fd

sayakpaul marked this pull request as ready for review November 13, 2023 14:47

sayakpaul added 6 commits November 14, 2023 07:34

imports

44be5fa

fix: imports.

b1de24c

Merge branch 'main' into refactor/loaders

c59774a

remove from main init for avoiding circular deps.

da4b4dc

remove spurious deps.

a3c402d

fix-copies.

19ea1e1

sayakpaul changed the title ~~[WIP][Refactor] refactor loaders.py to make it cleaner and leaner.~~ [Refactor] refactor loaders.py to make it cleaner and leaner. Nov 14, 2023

Merge branch 'main' into refactor/loaders

28d1256

fix imports.

2a86980

patrickvonplaten added 3 commits November 14, 2023 10:50

more debug

333ee0a

more debug

0e6430e

more debug

91b6af7

patrickvonplaten reviewed Nov 14, 2023

View reviewed changes

Comment thread src/diffusers/models/unet_2d_condition.py Outdated

patrickvonplaten reviewed Nov 14, 2023

View reviewed changes

Comment thread src/diffusers/models/unet_3d_condition.py Outdated

patrickvonplaten reviewed Nov 14, 2023

View reviewed changes

Comment thread src/diffusers/models/autoencoder_kl.py Outdated

patrickvonplaten added 2 commits November 14, 2023 12:04

Apply suggestions from code review

81cfe77

Apply suggestions from code review

b1bdd88

Merge branch 'main' into refactor/loaders

ba2aa58

patrickvonplaten merged commit ded93f7 into main Nov 14, 2023

sayakpaul mentioned this pull request Nov 14, 2023

[Refactor] Decouple the different loaders and create a separate loaders module #5669

Closed

stevhliu mentioned this pull request Nov 15, 2023

[docs] Loader APIs #5813

Merged

1 task

kashif deleted the refactor/loaders branch December 5, 2023 08:59

		@@ -0,0 +1,201 @@
		# Copyright 2023 The HuggingFace Team. All rights reserved.

Uh oh!

Conversation

sayakpaul commented Nov 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Uh oh!

Uh oh!

Uh oh!

patrickvonplaten Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

sayakpaul Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Nov 14, 2023

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

sayakpaul Nov 13, 2023

Choose a reason for hiding this comment

Uh oh!

patrickvonplaten left a comment • edited by sayakpaul Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

patrickvonplaten commented Nov 13, 2023

Uh oh!

HuggingFaceDocBuilderDev commented Nov 14, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sayakpaul commented Nov 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

patrickvonplaten commented Nov 14, 2023

Uh oh!

sayakpaul commented Nov 14, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sayakpaul commented Nov 12, 2023 •

edited

Loading

patrickvonplaten left a comment •

edited by sayakpaul

Loading

HuggingFaceDocBuilderDev commented Nov 14, 2023 •

edited

Loading