-
Couldn't load subscription status.
- Fork 6.5k
Test error raised when loading normal and expanding loras together in Flux #10188
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
Changes from 2 commits
cb98408
bbdbfaf
62ba78a
37b871c
e03084a
11fd809
7b5037f
ad180df
fc55809
4c0eb31
8083f91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -430,6 +430,97 @@ def test_correct_lora_configs_with_different_ranks(self): | |||||||||
| self.assertTrue(not np.allclose(original_output, lora_output_diff_alpha, atol=1e-3, rtol=1e-3)) | ||||||||||
| self.assertTrue(not np.allclose(lora_output_diff_alpha, lora_output_same_rank, atol=1e-3, rtol=1e-3)) | ||||||||||
|
|
||||||||||
| def test_lora_expanding_shape_with_normal_lora_raises_error(self): | ||||||||||
| # TODO: This test checks if an error is raised when a lora expands shapes (like control loras) but | ||||||||||
| # another lora with correct shapes is loaded. This is not supported at the moment and should raise an error. | ||||||||||
| # When we do support it, this test should be removed. Context: https://github.com/huggingface/diffusers/issues/10180 | ||||||||||
| components, _, _ = self.get_dummy_components(FlowMatchEulerDiscreteScheduler) | ||||||||||
| pipe = self.pipeline_class(**components) | ||||||||||
| pipe = pipe.to(torch_device) | ||||||||||
| pipe.set_progress_bar_config(disable=None) | ||||||||||
|
|
||||||||||
| logger = logging.get_logger("diffusers.loaders.lora_pipeline") | ||||||||||
| logger.setLevel(logging.DEBUG) | ||||||||||
|
|
||||||||||
| out_features, in_features = pipe.transformer.x_embedder.weight.shape | ||||||||||
| rank = 4 | ||||||||||
|
|
||||||||||
| shape_expander_lora_A = torch.nn.Linear(2 * in_features, rank, bias=False) | ||||||||||
| shape_expander_lora_B = torch.nn.Linear(rank, out_features, bias=False) | ||||||||||
| lora_state_dict = { | ||||||||||
| "transformer.x_embedder.lora_A.weight": shape_expander_lora_A.weight, | ||||||||||
| "transformer.x_embedder.lora_B.weight": shape_expander_lora_B.weight, | ||||||||||
| } | ||||||||||
| with CaptureLogger(logger) as cap_logger: | ||||||||||
| pipe.load_lora_weights(lora_state_dict, "adapter-1") | ||||||||||
| self.assertTrue(check_if_lora_correctly_set(pipe.transformer), "Lora not correctly set in denoiser") | ||||||||||
|
|
||||||||||
| self.assertTrue(pipe.transformer.x_embedder.weight.data.shape[1] == 2 * in_features) | ||||||||||
| self.assertTrue(pipe.transformer.config.in_channels == 2 * in_features) | ||||||||||
| self.assertTrue(cap_logger.out.startswith("Expanding the nn.Linear input/output features for module")) | ||||||||||
|
|
||||||||||
| normal_lora_A = torch.nn.Linear(in_features, rank, bias=False) | ||||||||||
| normal_lora_B = torch.nn.Linear(rank, out_features, bias=False) | ||||||||||
| lora_state_dict = { | ||||||||||
| "transformer.x_embedder.lora_A.weight": normal_lora_A.weight, | ||||||||||
| "transformer.x_embedder.lora_B.weight": normal_lora_B.weight, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| # The first lora expanded the input features of x_embedder. Here, we are trying to load a lora with the correct | ||||||||||
| # input features before expansion. This should raise an error about the weight shapes being incompatible. | ||||||||||
| self.assertRaisesRegex( | ||||||||||
| RuntimeError, | ||||||||||
| "size mismatch for x_embedder.lora_A.adapter-2.weight", | ||||||||||
| pipe.load_lora_weights, | ||||||||||
| lora_state_dict, | ||||||||||
| "adapter-2", | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| # Test the opposite case where the first lora has the correct input features and the second lora has expanded input features. | ||||||||||
| # This should raise a runtime error on input shapes being incompatible. But it doesn't. This is because PEFT renames the | ||||||||||
| # original layers as `base_layer` and the lora layers with the adapter names. This makes our logic to check if a lora | ||||||||||
| # weight is compatible with the current model incorrect. This should be addressed when attempting support for | ||||||||||
a-r-r-o-w marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| # https://github.com/huggingface/diffusers/issues/10180 (TODO) | ||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you provide some concrete LoCs as references for what you mean by:
Would also love to understand how this relates to how There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The lines in question are: diffusers/src/diffusers/loaders/lora_pipeline.py Lines 2319 to 2322 in d041dd5
When the first lora layer is loaded, assuming it is named After the first lora is loaded, peft updates the layer names to something like: [ So, when the second lora is loaded, it tries to find Note that I don't recall the exact layer names, so it may differ when you test and I'm just giving an example. The rough idea is that the current logic only works for loading:
For cases where we load shape expansion lora followed by normal lora, or vice versa, it will always fail currently. But as discussed in DM, this was not an anticipated use case - we only wanted to make control lora work as expected so the shape mismatch error when loading weights, instead of during inference where input shapes don't match, is OK for now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm got it. Thanks Aryan. Just noting this is enough for now. |
||||||||||
| components, _, _ = self.get_dummy_components(FlowMatchEulerDiscreteScheduler) | ||||||||||
| pipe = self.pipeline_class(**components) | ||||||||||
| pipe = pipe.to(torch_device) | ||||||||||
| pipe.set_progress_bar_config(disable=None) | ||||||||||
|
|
||||||||||
| logger = logging.get_logger("diffusers.loaders.lora_pipeline") | ||||||||||
| logger.setLevel(logging.DEBUG) | ||||||||||
|
|
||||||||||
| out_features, in_features = pipe.transformer.x_embedder.weight.shape | ||||||||||
| rank = 4 | ||||||||||
|
|
||||||||||
| lora_state_dict = { | ||||||||||
| "transformer.x_embedder.lora_A.weight": normal_lora_A.weight, | ||||||||||
| "transformer.x_embedder.lora_B.weight": normal_lora_B.weight, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| with CaptureLogger(logger) as cap_logger: | ||||||||||
| pipe.load_lora_weights(lora_state_dict, "adapter-1") | ||||||||||
| self.assertTrue(check_if_lora_correctly_set(pipe.transformer), "Lora not correctly set in denoiser") | ||||||||||
|
|
||||||||||
| self.assertTrue(pipe.transformer.x_embedder.weight.data.shape[1] == in_features) | ||||||||||
| self.assertTrue(pipe.transformer.config.in_channels == in_features) | ||||||||||
| self.assertFalse(cap_logger.out.startswith("Expanding the nn.Linear input/output features for module")) | ||||||||||
|
|
||||||||||
| lora_state_dict = { | ||||||||||
| "transformer.x_embedder.lora_A.weight": shape_expander_lora_A.weight, | ||||||||||
| "transformer.x_embedder.lora_B.weight": shape_expander_lora_B.weight, | ||||||||||
| } | ||||||||||
|
|
||||||||||
| # We should check for input shapes being incompatible here. But because above mentioned issue is | ||||||||||
| # not a supported use case, and because of the PEFT renaming, we will currently have a shape | ||||||||||
| # mismatch error. | ||||||||||
| self.assertRaisesRegex( | ||||||||||
| RuntimeError, | ||||||||||
| "size mismatch for x_embedder.lora_A.adapter-2.weight", | ||||||||||
| pipe.load_lora_weights, | ||||||||||
| lora_state_dict, | ||||||||||
| "adapter-2", | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| @unittest.skip("Not supported in Flux.") | ||||||||||
| def test_simple_inference_with_text_denoiser_block_scale_for_all_dict_options(self): | ||||||||||
| pass | ||||||||||
|
|
||||||||||
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.
Here I would run another inference round and make sure the outputs match with the LoRA that was correctly loaded. This will help us check if this loading error didn't leave the pipeline in a broken state, which is important.