-
Notifications
You must be signed in to change notification settings - Fork 6.6k
Fix LongCat LoRA load/unload and add regression test #12867
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
base: main
Are you sure you want to change the base?
Fix LongCat LoRA load/unload and add regression test #12867
Conversation
|
Hi @sayakpaul |
|
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. |
|
Hi @sayakpaul, |
|
|
||
|
|
||
| class LongCatImagePipeline(DiffusionPipeline, FromSingleFileMixin): | ||
| class LongCatImagePipeline(DiffusionPipeline, FluxLoraLoaderMixin, FromSingleFileMixin): |
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.
This seems quite incorrect to me.
Flux has two LoRA loadable modules:
diffusers/src/diffusers/loaders/lora_pipeline.py
Line 1493 in f6b6a71
| _lora_loadable_modules = ["transformer", "text_encoder"] |
For LongCat, it uses a different text encoder (Flux uses two text encoder, let along) and rest of its components also seems to be different from Flux:
diffusers/src/diffusers/pipelines/longcat_image/pipeline_longcat_image.py
Lines 214 to 222 in f6b6a71
| def __init__( | |
| self, | |
| scheduler: FlowMatchEulerDiscreteScheduler, | |
| vae: AutoencoderKL, | |
| text_encoder: Qwen2_5_VLForConditionalGeneration, | |
| tokenizer: Qwen2Tokenizer, | |
| text_processor: Qwen2VLProcessor, | |
| transformer: LongCatImageTransformer2DModel, | |
| ): |
So, could you please explain how using the FluxLoraLoaderMixin is appropriate here?
Instead, I suggest we write a dedicated LoRA loader mixin class for LongCat -- LongCatLoraLoaderMixin. You can refer to
diffusers/src/diffusers/loaders/lora_pipeline.py
Line 4775 in f6b6a71
| class QwenImageLoraLoaderMixin(LoraBaseMixin): |
as an example.
| @@ -0,0 +1,107 @@ | |||
| # Copyright 2025 The HuggingFace Team. | |||
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.
This is not needed. Please try to consult the existing testing structure for pipeline-level LoRA testing c.f. https://github.com/huggingface/diffusers/tree/main/tests/lora/
|
Thanks for the clarification @sayakpaul While implementing this, my reasoning was that So I’ll switch to a dedicated |
What does this PR do?
This PR fixes LoRA integration for
LongCatImagePipelineso thatload_lora_weights()properly applies the adapter during inference andunload_lora_weights()cleanly restores the base (non-LoRA) behavior.It also adds a slow regression test that:
Why?
Addresses the reported LoRA load/unload issue for LongCat:
Tests
RUN_SLOW=yes pytest -q tests/pipelines/longcat_image/test_longcat_lora.py