[Agent docs] some updated based on recent integration - #14452
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>
|
|
||
| **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. |
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`. |
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.
| ) | ||
| ``` | ||
|
|
||
| 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`. |
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?
| 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. |
There was a problem hiding this comment.
yeah but we want to refactor that pattern out
agree that it's confusing, I opened an TODO issue here #14469
i will remove this section for now, we can add it back once we did all the refactor
Co-authored-by: Steven Liu <59462357+stevhliu@users.noreply.github.com>
Reword the `_skip_keys` guidance: "non-tensor runtime objects holding large tensors" contradicted itself, say "state the model places itself" instead. Drop the paragraph claiming the guider spec is declared only by the denoise blocks — `WanTextEncoderStep` declares one too, and the codebase is inconsistent enough that no rule can be stated yet (see huggingface#14469). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No description provided.