huggingface / huggingface/diffusers

skyreels_v2 model/pipeline review

Abierto
#13,609 0 comentarios 0 reacciones 0 asignados Ver en GitHub
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

Abrir la 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

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.