huggingface / huggingface/diffusers
skyreels_v2 model/pipeline review
- Lenguaje dominante
- Python
- Estrellas
- 34.5k
- Forks
- 7.3k
- Merge medio
- 3 d 3 h
- PR fusionados (30 d)
- 91
Descripción
# `skyreels_v2` model/pipeline review
Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`
Review performed against the repository review rules. Reviewed public imports/lazy loading, transformer config/attention/runtime paths, all SkyReels-V2 pipelines, docs, examples, and tests. Duplicate searches found an existing duplicate only for the optional `ftfy` issue: https://github.com/huggingface/diffusers/issues/13112 and PR https://github.com/huggingface/diffusers/pull/13113.
Top-level imports passed in `.venv`. Full pytest collection could not run because this `.venv` torch build lacks `torch._C._distributed_c10d`, which the shared test mixins import through FSDP.
## Issue 1: Diffusion-forcing schedules drop tail latent frames
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing.py#L477-L480
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing.py#L563-L569
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_skyreels_v2.py#L657-L671
Problem:
`generate_timestep_matrix()` floors `num_latent_frames // causal_block_size` and later expands back to full frames. If the latent frame count is not divisible by the block size, the remainder is omitted. The documented 720P setting `base_num_frames=121` gives 31 latent frames with temporal scale 4, and `causal_block_size=5` covers only 30.
Impact:
The final latent frame can remain at initial noise or be skipped by the update schedule. Direct transformer calls with `num_frame_per_block > 1` also build masks assuming exact divisibility.
Reproduction:
```python
import torch
from diffusers import SkyReelsV2DiffusionForcingPipeline
num_latent_frames = (121 - 1) // 4 + 1
step_matrix, _, update_mask, intervals = SkyReelsV2DiffusionForcingPipeline.generate_timestep_matrix(
None, num_latent_frames, torch.arange(4), num_latent_frames, ar_step=0, causal_block_size=5
)
print(num_latent_frames, step_matrix.shape[1], update_mask.shape[1], intervals[-1])
# 31 30 30 (0, 30)
```
Relevant precedent:
The docs recommend the 121-frame 720P setting with causal block sizing here:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/docs/source/en/api/pipelines/skyreels_v2.md#L193-L197
Suggested fix:
Use ceil block counts and crop expanded tensors back to `num_latent_frames`. In the model mask, derive block IDs per actual frame instead of repeat-interleaving a floored block count:
```python
frame_ids = torch.arange(post_patch_num_frames, device=hidden_states.device)
block_ids = torch.div(frame_ids, self.config.num_frame_per_block, rounding_mode="floor")
causal_mask = block_ids.unsqueeze(0) <= block_ids.unsqueeze(1)
```
## Issue 2: Overlap noise uses global indices on local windows
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing.py#L875-L885
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing_i2v.py#L954-L964
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing_v2v.py#L962-L972
Problem:
`latent_model_input` is already sliced to `valid_interval_start:valid_interval_end`, but the overlap-noise branch slices it again with global indices. It also uses `torch.randn_like()` directly, bypassing the user-provided `generator`.
Impact:
Long-video overlap conditioning is noised on the wrong local frames, and `addnoise_condition > 0` is not reproducible from the pipeline generator.
Reproduction:
```python
import torch
prefix_video_latents_frames = 10
valid_interval_start = 5
valid_interval_end = 15
latent_model_input = torch.arange(valid_interval_start, valid_interval_end).view(1, 1, -1, 1, 1).float()
current = latent_model_input.clone()
current[:, :, valid_interval_start:prefix_video_latents_frames] = -1
expected = latent_model_input.clone()
expected[:, :, : prefix_video_latents_frames - valid_interval_start] = -1
print(current.flatten().tolist())
print(expected.flatten().tolist())
```
Relevant precedent:
Other diffusers latent preparation uses `randn_tensor(..., generator=generator)` for all user-visible randomness.
Suggested fix:
```python
local_prefix_end = min(prefix_video_latents_frames, valid_interval_end) - valid_interval_start
if addnoise_condition > 0 and local_prefix_end > 0:
noise_factor = 0.001 * addnoise_condition
prefix = latent_model_input[:, :, :local_prefix_end]
noise = randn_tensor(prefix.shape, generator=generator, device=prefix.device, dtype=prefix.dtype)
latent_model_input[:, :, :local_prefix_end] = prefix * (1.0 - noise_factor) + noise * noise_factor
timestep[:, :local_prefix_end] = addnoise_condition
```
## Issue 3: `causal_block_size` mutates serialized transformer config
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing.py#L781-L784
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_skyreels_v2.py#L765-L766
Problem:
A per-call pipeline option calls `self.transformer._set_ar_attention(causal_block_size)`, and `_set_ar_attention()` writes into the model config via `register_to_config`.
Impact:
One async call changes later calls that omit `causal_block_size`, and `save_pretrained()` will persist that call-time setting.
Reproduction:
```python
from diffusers import SkyReelsV2Transformer3DModel
m = SkyReelsV2Transformer3DModel(
patch_size=(1, 2, 2), num_attention_heads=2, attention_head_dim=12,
in_channels=4, out_channels=4, text_dim=16, freq_dim=256,
ffn_dim=32, num_layers=1, rope_max_seq_len=32,
)
print(m.config.num_frame_per_block)
m._set_ar_attention(5)
print(m.config.num_frame_per_block)
```
Relevant precedent:
Pipeline call arguments should not silently rewrite loadable model config.
Suggested fix:
Pass the block size as a runtime transformer forward argument, or temporarily restore the original config in a `try/finally` around the denoising loop.
## Issue 4: I2V input validation rejects valid image batches and accepts unusable embeds
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_i2v.py#L340-L341
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing_i2v.py#L319-L329
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing_i2v.py#L872-L872
Problem:
The I2V pipelines document `PipelineImageInput`, but reject a list of PIL images. The diffusion-forcing I2V pipeline also accepts `image_embeds` without `image`, then unconditionally preprocesses `image`.
Impact:
Documented batched image inputs fail early, while `image_embeds`-only calls pass validation and fail later.
Reproduction:
```python
import torch
from PIL import Image
from diffusers import SkyReelsV2ImageToVideoPipeline, SkyReelsV2DiffusionForcingImageToVideoPipeline
pipe = SkyReelsV2ImageToVideoPipeline.__new__(SkyReelsV2ImageToVideoPipeline)
pipe._callback_tensor_inputs = ["latents", "prompt_embeds", "negative_prompt_embeds"]
try:
pipe.check_inputs("a", None, [Image.new("RGB", (16, 16))], 16, 16, callback_on_step_end_tensor_inputs=["latents"])
except Exception as e:
print(type(e).__name__, e)
df_pipe = SkyReelsV2DiffusionForcingImageToVideoPipeline.__new__(SkyReelsV2DiffusionForcingImageToVideoPipeline)
df_pipe._callback_tensor_inputs = pipe._callback_tensor_inputs
df_pipe.check_inputs("a", None, None, 16, 16, image_embeds=torch.zeros(1, 1, 1), num_frames=9, base_num_frames=97)
print("image_embeds-only accepted")
```
Relevant precedent:
`PipelineImageInput` normally includes PIL images, tensors, and lists of them.
Suggested fix:
Allow list inputs in validation, and either remove `image_embeds` from the diffusion-forcing I2V public API or implement a real precomputed-conditioning path.
## Issue 5: Optional `ftfy` is still called unguarded
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2.py#L90-L92
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/skyreels_v2/pipeline_skyreels_v2_diffusion_forcing.py#L97-L100
Problem:
`ftfy` is imported only when available, but `basic_clean()` calls `ftfy.fix_text()` unconditionally.
Impact:
Prompt encoding crashes in environments where optional `ftfy` is not installed. This is already tracked in https://github.com/huggingface/diffusers/issues/13112 and PR https://github.com/huggingface/diffusers/pull/13113.
Reproduction:
```python
import diffusers.pipelines.skyreels_v2.pipeline_skyreels_v2 as m
m.__dict__.pop("ftfy", None)
m.prompt_clean("hello")
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/wan/pipeline_wan.py#L78-L82
Suggested fix:
```python
def basic_clean(text):
if is_ftfy_available():
text = ftfy.fix_text(text)
text = html.unescape(html.unescape(text))
return text.strip()
```
## Issue 6: Test coverage gaps hide SkyReels-V2 regressions
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/skyreels_v2/test_skyreels_v2_df_image_to_video.py#L37-L146
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/skyreels_v2/test_skyreels_v2.py#L31-L109
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/skyreels_v2/test_skyreels_v2_df_video_to_video.py#L38-L118
Problem:
`test_skyreels_v2_df_image_to_video.py` defines `SkyReelsV2DiffusionForcingImageToVideoPipelineFastTests` twice, so unittest discovery only exposes the second class name. The target family also has no `@slow` tests, and the fast tests mostly assert shapes with `max_diff <= 1e10`.
Impact:
The image-only DF I2V fixture is shadowed, official checkpoint behavior is untested, and regressions in output quality/scheduling can pass.
Reproduction:
```python
from pathlib import Path
import ast
files = list(Path("tests/pipelines/skyreels_v2").glob("test_*.py"))
print({p.name: "@slow" in p.read_text() for p in files})
p = Path("tests/pipelines/skyreels_v2/test_skyreels_v2_df_image_to_video.py")
classes = [n.name for n in ast.parse(p.read_text()).body if isinstance(n, ast.ClassDef)]
print(classes)
```
Relevant precedent:
Slow pipeline tests usually load a real small/official checkpoint path and assert deterministic output slices, not only output shapes.
Suggested fix:
Rename the second DF I2V test class, add slow tests for each public pipeline variant, and replace `1e10` thresholds with deterministic tensor slices or small visual/numeric checks.
## Issue 7: Docs reference a non-existent 1.3B 720P DF checkpoint
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/docs/source/en/api/pipelines/skyreels_v2.md#L221-L224
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/docs/source/en/api/pipelines/skyreels_v2.md#L284-L287
Problem:
The FLF2V and V2V examples use `Skywork/SkyReels-V2-DF-1.3B-720P-Diffusers`, but that repo does not exist. The supported list names `Skywork/SkyReels-V2-DF-1.3B-540P-Diffusers` and `Skywork/SkyReels-V2-DF-14B-720P-Diffusers`.
Impact:
Users copying the docs get a 404 before reaching pipeline execution.
Reproduction:
```python
from huggingface_hub import HfApi
api = HfApi()
api.model_info("Skywork/SkyReels-V2-DF-1.3B-720P-Diffusers")
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/docs/source/en/api/pipelines/skyreels_v2.md#L31-L39
Suggested fix:
Use `Skywork/SkyReels-V2-DF-14B-720P-Diffusers` for 720P examples, or change the example dimensions/base frames to the existing 1.3B 540P checkpoint.
Guía de contribución
Línea de trabajo
Comienza con los archivos de pipeline y transformer de SkyReels-V2 afectados en el commit 0f1abc4ae8b0eb2a3b40e82a310507281144c423 y, después, ejecuta las reproducciones proporcionadas de schedule, overlap-noise, config, validation y optional-ftfy. Revisa las pruebas de SkyReels-V2 y los ejemplos de documentación indicados para comprobar la cobertura y las referencias a checkpoints. Se considera terminado cuando las regresiones notificadas están cubiertas por pruebas significativas y la documentación y las entradas afectadas se comportan como se documenta.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- python, pytorch
- Área
- documentation, machine-learning, testing-qa
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Bastante claro
- Aptitud para principiantes
- 35/100