Skip to content

fix component library resolution when a transformers model folder shadows a pipeline dir - #14652

Open
akshan-main wants to merge 1 commit into
huggingface:mainfrom
akshan-main:fix-library-resolution-transformers-collision
Open

fix component library resolution when a transformers model folder shadows a pipeline dir#14652
akshan-main wants to merge 1 commit into
huggingface:mainfrom
akshan-main:fix-library-resolution-transformers-collision

Conversation

@akshan-main

Copy link
Copy Markdown
Contributor

What does this PR do?

_fetch_class_library_tuple treats a component as a pipeline-module class whenever its parent folder name matches a diffusers pipeline folder, without checking the module actually lives in diffusers. A transformers class whose model folder shares a name with a diffusers pipeline folder gets the pipeline folder recorded as its library in model_index.json, and from_pretrained then fails trying to import it from diffusers.pipelines.<name>.

This breaks DiffusionGemmaPipeline save/load on main today (transformers.models.diffusion_gemma collides with diffusers.pipelines.diffusion_gemma):

pipe.save_pretrained(d)  # model_index.json records ["diffusion_gemma", "DiffusionGemmaForBlockDiffusion"]
DiffusionGemmaPipeline.from_pretrained(d)
# AttributeError: module diffusers.pipelines.diffusion_gemma has no attribute DiffusionGemmaForBlockDiffusion

The fix requires the module path to start with the diffusers package for the pipeline-module branch. Classes genuinely defined inside a pipeline folder (e.g. IFWatermarker) still resolve to their pipeline dir, and unit tests cover all three resolution cases.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline?
  • Did you read our philosophy doc (important for complex PRs)?
  • Was this discussed/approved via a GitHub issue or the forum?
  • Did you make sure to update the documentation with your changes?
  • Did you write any new necessary tests?

Who can review?

@sayakpaul @DN6

@github-actions github-actions Bot added tests pipelines size/S PR with diff < 50 LOC labels Aug 30, 2026
@sayakpaul

Copy link
Copy Markdown
Member

Hmm interesting and this is not covered by our tests because DiffusionGemma tests follow its own testing suite (most regular pipeline tests don't apply to it).

@kashif I am wondering if it's possible to still extend the DiffusionGemma tests from the newly refactored mixis from tests/pipelines/testing_utils and mark the incompatible tests with xfail?

@sayakpaul
sayakpaul requested a review from yiyixuxu August 31, 2026 01:59
@github-actions

Copy link
Copy Markdown
Contributor

Hi @akshan-main, thanks for the PR! It does not appear to link an issue it fixes. If this PR addresses an existing issue, please add a closing keyword (e.g. Fixes #1234) to the PR description so the issue is linked. See the contribution guide for more details. If this PR intentionally does not fix a tracked issue, a maintainer can add the no-issue-needed label to silence this reminder.

Please note that PRs without a linked issue are likely to be automatically closed 10 days after this notice.

Once the PR links an issue (or gets the no-issue-needed label), you can ignore this message — it stays here as a comment, but it no longer applies.

@kashif

kashif commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Good idea, and it's doable. audioldm2 already does this for a non-image output (1-D waveform instead of (C,H,W)): it inherits BasePipelineTesterConfig and PipelineTesterMixin as-is and only xfails a couple of version-flaky tests. Most mixin tests (test_cfg, test_callback_cfg, test_num_images_per_prompt, test_encode_prompt_works_in_isolation) already self-skip when the pipeline lacks guidance_scale/num_images_per_prompt/encode_prompt, so DiffusionGemma wouldn't need much beyond that. Main work is building a tiny DiffusionGemmaForBlockDiffusion locally for get_dummy_components, same as transformers' own model tester does.

Want me to open a PR for this?

@sayakpaul

Copy link
Copy Markdown
Member

Want me to open a PR for this?

Yes please!

@kashif

kashif commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Opened #14661.

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

Labels

pipelines size/S PR with diff < 50 LOC tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants