Skip to content

Move LTX2Guidance Guider to guiders/ Folder - #14558

Open
dg845 wants to merge 5 commits into
mainfrom
ltx-25-guider-refactor
Open

Move LTX2Guidance Guider to guiders/ Folder#14558
dg845 wants to merge 5 commits into
mainfrom
ltx-25-guider-refactor

Conversation

@dg845

@dg845 dg845 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

This PR moves the LTX-2.X guider LTX2Guidance from src/diffusers/modular_pipelines/ltx2/guider.py to src/diffusers/guiders/ltx2_guidance.py. The main motivation is to make LTX-2.X modular repos which explicitly specify guiders loadable without an error (a hypothetical use case for this is shipping new guidance parameters with a finetuned LTX-2.X checkpoint).

Because LTX2Guidance currently lives in /modular_pipelines/ltx2, its type_hint will be resolved as ("ltx2", "LTX2Guidance") because /pipelines/ltx2 also exists. When we then load a guider configured with this type hint, the loading code then expects it to live at diffusers.pipelines.ltx2.LTX2Guidance, which causes an error to be raised when creating the modular pipeline.

This PR fixes this by moving LTX2Guidance to the guiders/ folder and making it importable from diffusers (like other guiders) so that its resulting type_hint resolves correctly.

Before submitting

  • Did you use an AI agent (Claude Code, Codex, Cursor, etc.) to help with this PR? If so:
    • Did you read the Coding with AI agents guide?
    • Did you run the self-review skill on the diff?
    • Did you share the final self-review notes in the PR description or a comment?
  • 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? Please add a link to it if that's the case.
  • Did you make sure to update the documentation with your changes? Here are the
    documentation guidelines, and
    here are tips on formatting docstrings.
  • Did you write any new necessary tests?
  • Are you the author (or part of the team) of the model/pipeline (only applicable for model/pipeline related PRs)?

Who can review?

Anyone in the community is free to review the PR once the tests have passed. Feel free to tag
members/contributors who may be interested in your PR.

@yiyixuxu
@sayakpaul

dg845 and others added 2 commits August 21, 2026 00:38
…LTX-2 guiders to be correctly loaded from checkpoint
…ponents

Covers the round trip this move fixes: a checkpoint that declares `guider` and
`audio_guider` in `modular_model_index.json` must record a resolvable
(library, class_name) pair and reload its non-default guidance scales. The
existing `test_modular_index_consistency` skips components without a
`pretrained_model_name_or_path`, so `from_config` guiders were uncovered.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/M PR with diff < 200 LOC documentation Improvements or additions to documentation tests modular-pipelines utils guiders labels Aug 21, 2026
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

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.

@dg845

dg845 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author
Self-Review Report

Self-review

Ran the repo's self-review rubric (.ai/review-rules.md, plus .ai/AGENTS.md, .ai/modular.md, .ai/testing.md) over the full branch diff — 7 files, +68/-8.

Verdict: READY — no blocking issues, no dead code to remove.

Non-blocking issues

1. One assertion tests overwrite_modular_index, not the regression. tests/modular_pipelines/ltx2/test_modular_pipeline_ltx2.py:197

assert index[name][2]["pretrained_model_name_or_path"] == str(repo) pins the repointing behavior of overwrite_modular_index=True rather than class resolution. Defensible — it's what makes a published checkpoint self-contained, and nothing else in the suite covers it — but it couples this test to a second feature. Happy to drop it if you'd rather keep the test single-purpose.

2. Placement: LTX-2-specific vs. generalized. The failure mode — a from_config component promoted to from_pretrained by a repo — is generic; only the class-location trigger is LTX-specific. This could be lifted into ModularLoadingTesterMixin so every modular pipeline with a guider is covered. I kept it targeted because generalizing means handling pipelines with no guider or an already-from_pretrained one, and would add a full extra save_pretrained to every modular suite.

3. The design note sits in a public module where autodoc won't render it. src/diffusers/guiders/ltx2_guidance.py:15-32

The module-level comment carries the substantive explanation of how the guider is driven, but the autodoc directive renders the class docstring (:48), so none of it reaches the published page. The audience also shifted with the move — the "Parity note" paragraph addresses whoever maintains the LTX-2 parity tests, which reads oddly in a general-purpose public module. Possible follow-up: lift the API-usage half into the class docstring.

4. MagnitudeAwareGuidance has the same latent bug. It's exported from src/diffusers/guiders/__init__.py:27 but absent from both top-level export sites in src/diffusers/__init__.py, so hasattr(diffusers, "MagnitudeAwareGuidance") is False and any repo declaring it as a from_pretrained component would fail to load the same way. Out of scope here — flagging so it isn't lost. One-line fix if we want it in this PR.

Suggestions

5. Record the rule in .ai/modular.md, near the guider guidance at line 301: a new guider belongs in src/diffusers/guiders/ and must be exported from the top-level namespace, because _fetch_class_library_tuple derives the library from the module path and modular_pipelines/<model>/ collides with pipelines/<model>/.

6. LTX2Guidance is now a public, documented guider that can't be swapped. LTX2LoopDenoiser reads guider.spatio_temporal_guidance_blocks, so substituting a generic guider raises AttributeError: 'ClassifierFreeGuidance' object has no attribute 'spatio_temporal_guidance_blocks' (verified). Pre-existing and arguably intended — the block pins the type via ComponentSpec("guider", LTX2Guidance, ...) — but a sentence in the class docstring would set expectations now that it sits in the public guider lineup.

Dead code (advisory)

path:line verdict reason
guiders/ltx2_guidance.py:128 prepare_inputs Used Implements the abstract contract (guider_utils.py:168 raises NotImplementedError) and is the documented modular loop pattern. The LTX-2 denoiser uses the block-state variant, but the override is required API.
guiders/ltx2_guidance.py:103 active_predictions Used Drives both prepare methods.

No new code paths — the source diff is a relocation plus export wiring.

Notes for the reviewer

  • ModularGuiderTesterMixin is absent from the LTX-2 test classes even though .ai/testing.md prescribes it for any pipeline with a guider. That omission is justified rather than an oversight — the mixin hardcodes a ClassifierFreeGuidance swap, which the LTX-2 denoiser rejects (see suggestion 6).
  • The regression test is proven to bite. With LTX2Guidance.__module__ reverted to the pre-fix path it fails on the type_hint assertion; removing the top-level export fails the test module at import.
  • make repo-consistency fails on this branch, but not because of it. check_repo.py imports the nonexistent diffusers.models.auto and check_inits.py looks for src/transformers/__init__.py; both fail before reading any source, identically on a clean tree.

Verification

  • TestLTX2Text2VideoModularPipelineLoading — 9 passed
  • Full LTX-2 modular suite — 163 passed, 10 skipped
  • make style, make quality, check_dummies, check_copies — clean
  • Save/load round trip on an unpatched repo: index records ("diffusers", "LTX2Guidance") and reloads non-default scales

@dg845
dg845 requested a review from yiyixuxu August 22, 2026 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation guiders modular-pipelines size/M PR with diff < 200 LOC tests utils

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants