[Agent docs] some updated based on recent integration - #14452
Open
yiyixuxu wants to merge 1 commit into
Open
Conversation
- models.md: document _skip_keys (group offloading exclude_kwargs + device_map dispatch skip_keys) with the in-tree cache examples - modular.md: canonical flat-blockset packing (standalone children, InsertableDict groups, no cross-preset-file imports); variant presets carry their own model_name + mapping entry (huggingface#14451); composed-blockset inputs/outputs overrides; guider ownership and requires_unconditional_embeds; always ship modular_model_index.json; auto-docstring drift + check_forward_call_docstrings checklist items; randn_tensor gotcha - pipelines.md: the same randn_tensor gotcha Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
sayakpaul
approved these changes
Aug 12, 2026
|
|
||
| **API:** every device-placement hook that moves call inputs — `apply_group_offloading(model, ...)` reads it as `exclude_kwargs` (`hooks/group_offloading.py`), and `from_pretrained(..., device_map=...)` (including the `device_map={"": "cpu"}` offload placement) forwards it to accelerate's `dispatch_model(skip_keys=...)` (`pipelines/pipeline_loading_utils.py`). | ||
|
|
||
| These hooks move every tensor in the call's args/kwargs to the execution device before `forward` runs. List the forward kwargs that carry non-tensor runtime objects holding large tensors — a KV-cache, an encoder feature cache — so the hooks leave them alone instead of transferring (or repeatedly re-transferring) their contents on every call. |
Member
There was a problem hiding this comment.
non-tensor runtime objects holding large tensors
This sounds a bit confusing to me. Like non-tensor objects holding large tensors feels like we're contradicting in scope.
| ``` | ||
| The bug is invisible until someone actually passes `callback_on_step_end` — the `PipelineTesterMixin` callback tests are what catch it. | ||
|
|
||
| 10. **Raw `torch.randn(device=...)` for noise.** Use `randn_tensor(...)` from `utils/torch_utils`: it draws on the generator's device and moves the result, so CPU generators work, and the CUDA-generator path is bit-identical to `torch.randn`. |
Member
There was a problem hiding this comment.
In a future PR we could separate out the common bits shared between standard and modular pipelines in a different file.
stevhliu
approved these changes
Aug 12, 2026
| This section provides guidance on how to execute pipelines and blocks — in scripts, debugging sessions, and tests alike. | ||
|
|
||
| - **Full pipeline from a repo**: `ModularPipeline.from_pretrained(repo_id)` — the base class, not the model subclass; it resolves the right class from the repo's `modular_model_index.json` (falling back to a standard `model_index.json`). Then `pipe.load_components()` and call it. | ||
| - **Full pipeline from a repo**: `ModularPipeline.from_pretrained(repo_id)` — the base class, not the model subclass; it resolves the right class from the repo's `modular_model_index.json` (falling back to a standard `model_index.json`). Then `pipe.load_components()` and call it. A repo meant for `ModularPipeline.from_pretrained` should always ship a `modular_model_index.json` (its `_class_name` routes to the right pipeline class); the `model_index.json` fallback is a last resort with limited support. |
Member
There was a problem hiding this comment.
Suggested change
| - **Full pipeline from a repo**: `ModularPipeline.from_pretrained(repo_id)` — the base class, not the model subclass; it resolves the right class from the repo's `modular_model_index.json` (falling back to a standard `model_index.json`). Then `pipe.load_components()` and call it. A repo meant for `ModularPipeline.from_pretrained` should always ship a `modular_model_index.json` (its `_class_name` routes to the right pipeline class); the `model_index.json` fallback is a last resort with limited support. | |
| - **Full pipeline from a repo**: `ModularPipeline.from_pretrained(repo_id)` — the base class, not the model subclass; it resolves the right class from the repo's `modular_model_index.json` (falling back to a standard `model_index.json`). Then `pipe.load_components()` and call it. New modular repositories should include `modular_model_index.json` because it records modular block and component metadata. `model_index.json` remains supported for compatibility with standard repositories, but it cannot express all modular metadata. |
| ) | ||
| ``` | ||
|
|
||
| The guider spec is declared **only by the denoise blocks** — each preset's denoise step pins its scale there (base `FrozenDict({"guidance_scale": 3.0})`, distilled `1.0`), which is how one repo's checkpoint gets the right guidance with no `guidance_scale` input anywhere. The text encoder never declares a guider: it asks `components.requires_unconditional_embeds` (a pipeline-class property that consults the guider when one exists) to decide whether to encode the negative prompt, defaulting it to `""`; an explicitly passed `negative_prompt` is the standalone escape hatch. See `wan_animate_2/encoders.py` / `denoise.py`. |
Member
There was a problem hiding this comment.
maybe soften a bit here about guider specs only being declared by the denoise blocks since it seems like WanTextEncoderStep does declare it as well?
Suggested change
| The guider spec is declared **only by the denoise blocks** — each preset's denoise step pins its scale there (base `FrozenDict({"guidance_scale": 3.0})`, distilled `1.0`), which is how one repo's checkpoint gets the right guidance with no `guidance_scale` input anywhere. The text encoder never declares a guider: it asks `components.requires_unconditional_embeds` (a pipeline-class property that consults the guider when one exists) to decide whether to encode the negative prompt, defaulting it to `""`; an explicitly passed `negative_prompt` is the standalone escape hatch. See `wan_animate_2/encoders.py` / `denoise.py`. | |
| Declare the guider spec in every block that requires it, typically the text encoder and denoise blocks, using the same configuration so the assembled pipeline shares one guider. The text encoder can use `components.requires_unconditional_embeds` to decide whether to produce negative embeddings. |
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
3 participants
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.