Skip to content

Warn and skip transformer LoRA loading when the transformer is absent (#13487)#13945

Open
archievi wants to merge 1 commit into
huggingface:mainfrom
archievi:fix/lora-warn-skip-missing-transformer-13487
Open

Warn and skip transformer LoRA loading when the transformer is absent (#13487)#13945
archievi wants to merge 1 commit into
huggingface:mainfrom
archievi:fix/lora-warn-skip-missing-transformer-13487

Conversation

@archievi

Copy link
Copy Markdown

What does this PR do?

Loading a transformer-only LoRA onto a Modular sub-pipeline that does not contain the transformer (for example a text-encoder-only text_pipe) currently crashes:

AttributeError: 'Flux2ModularPipeline' object has no attribute 'transformer'

This happens because load_lora_weights resolves the transformer with getattr(self, self.transformer_name), which raises when the (sub-)pipeline has no transformer component.

Following @yiyixuxu's suggestion on the issue ("maybe warn and skip is better"), load_lora_weights now resolves the transformer with a None default and, when no transformer is present, logs a warning and skips loading the transformer LoRA layers instead of raising.

The change is made at the copied-from source (CogVideoXLoraLoaderMixin.load_lora_weights) and propagated to its 14 copies with make fix-copies, so the behavior is consistent across the transformer LoRA loaders (Flux2, Flux, SD3, CogView4, etc.). Normal pipelines that do have a transformer are unaffected.

Fixes #13487

Before submitting

  • Did you use an AI agent (Claude Code, Codex, Cursor, etc.) to help with this PR? If so:
    • Did you point it at the project conventions in .ai/? I read and applied .ai/AGENTS.md (copied-code convention) and .ai/modular.md; the fix is made at the copied-from source and propagated with make fix-copies rather than editing individual copies.
    • Did you self-review the diff against .ai/review-rules.md? No ephemeral context, correctness-focused, no dead code.
  • Did you read the contributor guideline?
  • Did you read our philosophy doc? (small, localized fix)
  • Was this discussed/approved via a GitHub issue? Yes — Loading loras on "text_pipe" of a modular pipeline fails #13487, where @yiyixuxu suggested the warn-and-skip approach and invited a PR.
  • Did you make sure to update the documentation with your changes? (no user-facing API/doc change)
  • Did you write any new necessary tests? Added Flux2LoRAModularPipelineTests in tests/lora/test_lora_layers_flux2.py.

Verification (run locally)

  • New regression test passes, and it reproduces the original AttributeError when the fix is reverted (regression-proven).
  • python utils/check_copies.py passes (copies consistent after make fix-copies).
  • ruff check and ruff format --check are clean on the changed files.

Who can review?

@yiyixuxu @sayakpaul @DN6

…huggingface#13487)

Loading a transformer-only LoRA onto a modular sub-pipeline that does not contain
the transformer (for example a text-encoder-only sub-pipeline) raised
"AttributeError: ... object has no attribute 'transformer'".

Following the maintainer suggestion on the issue, load_lora_weights now resolves the
transformer with a default of None and, when it is missing, logs a warning and skips
loading the transformer LoRA layers instead of raising. The change is made at the
copied-from source (CogVideoXLoraLoaderMixin.load_lora_weights) and propagated to the
copies with make fix-copies. Adds a regression test.

@sayakpaul sayakpaul left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I left some comments.

Comment on lines +1384 to +1390
transformer = self.transformer if hasattr(self, "transformer") else getattr(self, self.transformer_name, None)
if transformer is None:
logger.warning(
f"No `{self.transformer_name}` module was found in {self.__class__.__name__}, so no LoRA weights will be "
"loaded into the transformer. This can happen when loading LoRA weights into a modular pipeline that "
"does not contain the transformer (for example, a text-encoder-only sub-pipeline)."
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not specific to a modular pipeline, though.



@require_peft_backend
class Flux2LoRAModularPipelineTests(unittest.TestCase):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should be added to

class PeftLoraLoaderMixinTests:
as I don't think the underlying issue only applies to modular pipelines.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Loading loras on "text_pipe" of a modular pipeline fails

2 participants