Repository navigation
Conversation
… for base functionality Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
stevhliu
approved these changes
Nov 12, 2026
|
|
||
| Conversion ends with a repo a user can load in one call. Decide its layout with the user before the conversion script writes anything — the script's output directory should already be that layout. | ||
|
|
||
| **Separate repo or the reference checkpoint's repo — the user's call.** In a separate repo the converted weights are laid out the standard way (`transformer/`, `vae/`, `text_encoder/`, ...). In the reference checkpoint's repo the converted components sit next to the reference ones, and the only practical difference is folder names: when the reference checkpoint already uses a name the diffusers layout needs (`transformer/`, `vae/`, ...), write the converted component to `<name>_diffusers/` and leave the reference folder untouched. [MiniMaxAI/MiniMax-H3-Image](https://huggingface.co/MiniMaxAI/MiniMax-H3-Image) is the worked example: `transformer/` and `vae/` hold the reference weights, `transformer_diffusers/` and `vae_diffusers/` hold the converted ones, and `modular_model_index.json` points at the latter through each component's `subfolder`. Components with no name clash keep their plain folder name. |
Member
There was a problem hiding this comment.
does the <name>_diffusers/ "trick" work with a standard DiffusionPipeline as well? may be nice to clarify here to avoid the agent following this section building a repo that won't load
|
|
||
| **Separate repo or the reference checkpoint's repo — the user's call.** In a separate repo the converted weights are laid out the standard way (`transformer/`, `vae/`, `text_encoder/`, ...). In the reference checkpoint's repo the converted components sit next to the reference ones, and the only practical difference is folder names: when the reference checkpoint already uses a name the diffusers layout needs (`transformer/`, `vae/`, ...), write the converted component to `<name>_diffusers/` and leave the reference folder untouched. [MiniMaxAI/MiniMax-H3-Image](https://huggingface.co/MiniMaxAI/MiniMax-H3-Image) is the worked example: `transformer/` and `vae/` hold the reference weights, `transformer_diffusers/` and `vae_diffusers/` hold the converted ones, and `modular_model_index.json` points at the latter through each component's `subfolder`. Components with no name clash keep their plain folder name. | ||
|
|
||
| **Self-contained by default.** Every component's loading spec in the index points at this repo, including a component that is identical to another model's (a shared text encoder or VAE). Don't reference another repo just to save disk space: the Hub and the local cache already deduplicate identical files. See [Referencing components in other repositories](https://huggingface.co/docs/diffusers/main/en/modular_diffusers/modular_pipeline#referencing-components-in-other-repositories). |
Member
There was a problem hiding this comment.
Suggested change
| **Self-contained by default.** Every component's loading spec in the index points at this repo, including a component that is identical to another model's (a shared text encoder or VAE). Don't reference another repo just to save disk space: the Hub and the local cache already deduplicate identical files. See [Referencing components in other repositories](https://huggingface.co/docs/diffusers/main/en/modular_diffusers/modular_pipeline#referencing-components-in-other-repositories). | |
| **Self-contained by default.** Every component's loading spec in the index points at this repo, including a component that is identical to another model's (a shared text encoder or VAE). Don't reference another repo just to save disk space. Identical Xet-backed files share one cached copy across repos (See [Share files across repositories](https://huggingface.co/docs/diffusers/main/en/installation#share-files-across-repositories)). |
| - Keep component sizes tiny so the suite runs fast — small `num_layers`, small hidden/attention dims, low resolution, few frames. Reference `tests/pipelines/wan/test_wan.py` (`get_dummy_components` and `get_dummy_inputs`) for the size scale to target. | ||
| - Build dummy components from the **real classes** at tiny config — a real VAE with tiny dims, a real tokenizer from an `hf-internal-testing/tiny-random-*` repo. Don't substitute a hand-rolled mock (a bare `nn.Module` with a `SimpleNamespace` config, a fake tokenizer) without a good reason: a mock is written by copying whatever the pipeline reads from the component today, so it can only confirm the pipeline against itself — the test stays green when the component renames a config field or the pipeline starts reading one the component doesn't have, and catching exactly that pipeline↔component contract is what a pipeline test is for. A good reason to stub: the component is impractical to instantiate and only its I/O matters to the pipeline (e.g. `DummyCosmosSafetyChecker` standing in for the huge Cosmos guardrail) — then make it a shared, purpose-built class honoring the real interface. | ||
| - The same applies to test doubles at the call level: don't monkeypatch a component method (e.g. the scheduler's `set_timesteps`) just to capture what the code under test passed to it — that only verifies the caller against itself, not against the real method's contract. Call the real component and assert on its resulting state. | ||
| - Don't test base functionality per model or per pipeline. Behavior every model or pipeline shares (`from_pretrained` / `save_pretrained`, `_keys_to_ignore_on_load_unexpected`, offloading, ...) belongs in the shared tester mixin, where every model runs it. If the mixin doesn't cover it yet, open an issue asking for it rather than writing a one-off test in the model's file. |
Member
There was a problem hiding this comment.
don't want to contradict CLAUDE.md which doesn't allow posting to GitHub directly
Suggested change
| - Don't test base functionality per model or per pipeline. Behavior every model or pipeline shares (`from_pretrained` / `save_pretrained`, `_keys_to_ignore_on_load_unexpected`, offloading, ...) belongs in the shared tester mixin, where every model runs it. If the mixin doesn't cover it yet, open an issue asking for it rather than writing a one-off test in the model's file. | |
| - Don't test base functionality per model or per pipeline. Behavior every model or pipeline shares (`from_pretrained` / `save_pretrained`, `_keys_to_ignore_on_load_unexpected`, offloading, ...) belongs in the shared tester mixin, where every model or pipeline runs it. If the mixin doesn’t cover it yet, tell the user so they can open an issue, rather than writing a one-off test in the model’s file. |
|
|
||
| ## Hub checkpoint | ||
|
|
||
| Conversion ends with a repo a user can load in one call. Decide its layout with the user before the conversion script writes anything — the script's output directory should already be that layout. |
Member
There was a problem hiding this comment.
Suggested change
| Conversion ends with a repo a user can load in one call. Decide its layout with the user before the conversion script writes anything — the script's output directory should already be that layout. | |
| Conversion ends with a repo that users can load with `ModularPipeline.from_pretrained`. Decide its layout with the user before the conversion script writes anything — the script's output directory should already be that layout. |
sayakpaul
approved these changes
Nov 13, 2026
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.