huggingface / huggingface/diffusers
ovis_image 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
# `ovis_image` model/pipeline review
Commit tested: `0f1abc4ae8b0eb2a3b40e82a310507281144c423`
Review performed against the repository review rules. `AGENTS.md` is referenced by `.ai/review-rules.md` but is not present in this checkout; the other referenced rule files were read and applied.
Files/categories reviewed: target pipeline/model files, public imports and lazy loading, config/serialization, dtype/device/offload paths, attention processor behavior, docs, and fast/slow test coverage.
Duplicate search status: `gh search` hit the GitHub API rate limit, so I checked GitHub web issue/PR searches for `OvisImage`, `ovis_image`, `OvisImageTransformer2DModel AttentionMixin`, and `OvisImagePipeline num_images_per_prompt`. I did not find an exact Ovis duplicate. Related but not duplicate: https://github.com/huggingface/diffusers/issues/12186 covers the same missing-`AttentionMixin` pattern for `WanVACETransformer3DModel`.
## Issue 1: Transformer does not expose attention processor APIs
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_ovis_image.py#L22-L28
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_ovis_image.py#L386-L392
Problem:
`OvisImageTransformer2DModel` defines `OvisImageAttention` modules but does not inherit `AttentionMixin`. That leaves the model without the standard `attn_processors`, `set_attn_processor`, `fuse_qkv_projections`, and `unfuse_qkv_projections` APIs expected by related transformer families.
Impact:
Users and tests cannot swap attention processors, inspect processors, or use QKV fusion through the model-level API.
Reproduction:
```python
from diffusers import OvisImageTransformer2DModel
print(hasattr(OvisImageTransformer2DModel, "set_attn_processor"))
print(hasattr(OvisImageTransformer2DModel, "fuse_qkv_projections"))
assert hasattr(OvisImageTransformer2DModel, "set_attn_processor")
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_flux.py#L28-L33
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_flux.py#L525-L533
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_qwenimage.py#L29
Suggested fix:
```python
from ..attention import AttentionMixin, AttentionModuleMixin, FeedForward
class OvisImageTransformer2DModel(
ModelMixin,
ConfigMixin,
PeftAdapterMixin,
FromOriginalModelMixin,
CacheMixin,
AttentionMixin,
):
...
```
## Issue 2: `joint_attention_kwargs` is accepted but never reaches attention
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L519-L590
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L605-L624
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_ovis_image.py#L478-L486
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_ovis_image.py#L535-L569
Problem:
The pipeline exposes `joint_attention_kwargs`, but `OvisImageTransformer2DModel.forward()` does not accept it, and the pipeline transformer calls do not pass it. The block classes already accept `joint_attention_kwargs`, so the plumbing is incomplete.
Impact:
Any user-provided attention kwargs are silently ignored by the pipeline. Direct model calls with the same argument fail.
Reproduction:
```python
import inspect
from diffusers import OvisImagePipeline, OvisImageTransformer2DModel
print("pipeline:", "joint_attention_kwargs" in inspect.signature(OvisImagePipeline.__call__).parameters)
print("model:", "joint_attention_kwargs" in inspect.signature(OvisImageTransformer2DModel.forward).parameters)
assert "joint_attention_kwargs" in inspect.signature(OvisImageTransformer2DModel.forward).parameters
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_flux.py#L647-L648
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/models/transformers/transformer_flux.py#L722-L731
Suggested fix:
```python
def forward(..., joint_attention_kwargs: dict[str, Any] | None = None, return_dict: bool = True):
...
encoder_hidden_states, hidden_states = block(
hidden_states=hidden_states,
encoder_hidden_states=encoder_hidden_states,
temb=temb,
image_rotary_emb=image_rotary_emb,
joint_attention_kwargs=joint_attention_kwargs,
)
```
Also pass `joint_attention_kwargs=self.joint_attention_kwargs` in both pipeline transformer calls.
## Issue 3: `guidance_scale` property is never initialized
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L393-L394
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L519-L521
Problem:
`OvisImagePipeline.guidance_scale` returns `self._guidance_scale`, but `__call__` never assigns `self._guidance_scale = guidance_scale`.
Impact:
Callbacks or downstream code that read `pipe.guidance_scale` during generation can hit an `AttributeError` or stale state, unlike related pipelines.
Reproduction:
```python
import inspect
from diffusers import OvisImagePipeline
source = inspect.getsource(OvisImagePipeline.__call__)
print("self._guidance_scale = guidance_scale" in source)
assert "self._guidance_scale = guidance_scale" in source
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/flux/pipeline_flux.py#L802-L804
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/qwenimage/pipeline_qwenimage.py#L588-L590
Suggested fix:
```python
self._guidance_scale = guidance_scale
self._joint_attention_kwargs = joint_attention_kwargs
self._current_timestep = None
self._interrupt = False
```
## Issue 4: Batched prompts break with default negative prompt under CFG
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L310-L314
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L523-L552
Problem:
With `prompt` as a list and default `negative_prompt=""`, positive embeddings are batched to `len(prompt)`, but negative embeddings are encoded as batch size 1. CFG then calls the transformer with mismatched latent and negative prompt batch sizes.
Impact:
The default CFG path fails for normal batched text-to-image usage unless users manually pass a negative prompt list of matching length.
Reproduction:
```python
import torch
from diffusers import OvisImagePipeline
pipe = OvisImagePipeline.__new__(OvisImagePipeline)
pipe.text_encoder = type("E", (), {"dtype": torch.float32})()
pipe.transformer = type("T", (), {"dtype": torch.float32})()
def fake_get_ovis_prompt_embeds(prompt, num_images_per_prompt=1, device=None, dtype=None):
prompt = [prompt] if isinstance(prompt, str) else prompt
return torch.zeros(len(prompt) * num_images_per_prompt, 4, 8)
pipe._get_ovis_prompt_embeds = fake_get_ovis_prompt_embeds
pos, _ = pipe.encode_prompt(["cat", "dog"], device=torch.device("cpu"))
neg, _ = pipe.encode_prompt("", device=torch.device("cpu"))
print(pos.shape[0], neg.shape[0])
assert pos.shape[0] == neg.shape[0]
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/aura_flow/pipeline_aura_flow.py#L335-L360
Suggested fix:
```python
if do_classifier_free_guidance and negative_prompt_embeds is None:
if negative_prompt is None:
negative_prompt = ""
if isinstance(negative_prompt, str):
negative_prompt = [negative_prompt] * batch_size
elif len(negative_prompt) != batch_size:
raise ValueError(
f"`negative_prompt` has batch size {len(negative_prompt)}, but `prompt` has batch size {batch_size}."
)
```
## Issue 5: Precomputed `prompt_embeds` are not moved or repeated
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L240-L274
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L554-L565
Problem:
When `prompt_embeds` is supplied, `encode_prompt()` does not move it to the execution device/dtype and does not repeat it for `num_images_per_prompt`. The pipeline still prepares latents for `batch_size * num_images_per_prompt`.
Impact:
Precomputed embeddings can fail with device mismatches on GPU/offload paths and batch mismatches when generating multiple images per prompt.
Reproduction:
```python
import torch
from diffusers import OvisImagePipeline
pipe = OvisImagePipeline.__new__(OvisImagePipeline)
pipe.text_encoder = None
pipe.transformer = type("T", (), {"dtype": torch.float16})()
embeds = torch.randn(1, 4, 8, dtype=torch.float32)
out, ids = pipe.encode_prompt(None, device=torch.device("meta"), prompt_embeds=embeds, num_images_per_prompt=2)
print(out.shape, out.device, ids.device)
assert out.shape[0] == 2
assert out.device == ids.device
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/qwenimage/pipeline_qwenimage.py#L250-L264
Suggested fix:
```python
batch_size = len(prompt) if prompt_embeds is None else prompt_embeds.shape[0]
if prompt_embeds is None:
prompt_embeds = self._get_ovis_prompt_embeds(...)
dtype = self.text_encoder.dtype if self.text_encoder is not None else self.transformer.dtype
prompt_embeds = prompt_embeds.to(device=device, dtype=dtype)
_, seq_len, _ = prompt_embeds.shape
prompt_embeds = prompt_embeds.repeat(1, num_images_per_prompt, 1)
prompt_embeds = prompt_embeds.view(batch_size * num_images_per_prompt, seq_len, -1)
```
## Issue 6: `max_sequence_length` is validated but ignored
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L201-L229
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L240-L274
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py#L432-L552
Problem:
`__call__` accepts and validates `max_sequence_length`, but `encode_prompt()` has no such parameter and `_get_ovis_prompt_embeds()` always tokenizes with `self.tokenizer_max_length`.
Impact:
Users cannot reduce prompt sequence length for speed/memory, and the public argument is misleading.
Reproduction:
```python
import inspect
from diffusers import OvisImagePipeline
assert "max_sequence_length" in inspect.signature(OvisImagePipeline.__call__).parameters
assert "max_sequence_length" in inspect.signature(OvisImagePipeline.encode_prompt).parameters
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/src/diffusers/pipelines/qwenimage/pipeline_qwenimage.py#L226-L264
Suggested fix:
```python
def _get_ovis_prompt_embeds(..., max_sequence_length: int = 256):
max_length = max_sequence_length + self.user_prompt_begin_id
tokens = self.tokenizer(..., max_length=max_length, ...)
prompt_embeds = prompt_embeds[:, self.user_prompt_begin_id : self.user_prompt_begin_id + max_sequence_length, :]
def encode_prompt(..., max_sequence_length: int = 256):
...
```
Then pass `max_sequence_length=max_sequence_length` from `__call__`.
## Issue 7: No fast or slow tests cover `ovis_image`
Affected code:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/ovis_image/__init__.py#L1
Problem:
`tests/pipelines/ovis_image/` only contains an empty `__init__.py`, and there is no model test for `OvisImageTransformer2DModel`. No fast or slow tests reference `OvisImage` or `ovis_image`.
Impact:
The import surface, prompt batching, callback properties, attention APIs, serialization, and slow checkpoint path can regress without CI coverage.
Reproduction:
```python
from pathlib import Path
paths = []
for path in Path("tests").rglob("test*.py"):
text = path.read_text(encoding="utf-8", errors="ignore")
if "OvisImage" in text or "ovis_image" in text or "ovis-image" in text.lower():
paths.append(str(path))
print(paths)
assert paths, "No fast or slow Ovis tests found"
```
Relevant precedent:
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/models/transformers/test_models_transformer_qwenimage.py#L43-L79
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/qwenimage/test_qwenimage.py#L36-L120
https://github.com/huggingface/diffusers/blob/0f1abc4ae8b0eb2a3b40e82a310507281144c423/tests/pipelines/flux/test_pipeline_flux.py#L240-L303
Suggested fix:
Add a model fast test using `ModelTesterMixin` and `AttentionTesterMixin`, add a pipeline fast test with tiny synthetic components, and add at least one `@slow` pipeline smoke test for the published Ovis checkpoint. These tests should cover top-level imports, save/load, attention processor APIs, batched prompts with CFG, `prompt_embeds`, callback properties, and `max_sequence_length`.
Guía de contribución
Línea de trabajo
Start with src/diffusers/pipelines/ovis_image/pipeline_ovis_image.py and src/diffusers/models/transformers/transformer_ovis_image.py, then compare the linked Flux and Qwen Image implementations. Run the supplied reproductions and inspect existing Ovis Image tests and fast/slow coverage. Done means all six reported API, state, batching, embedding, and sequence-length behaviors are implemented and covered by tests.
Escrito por el modelo de indexación a partir del texto del issue.
Evaluación
- Stack tecnológico
- python, pytorch
- Área
- machine-learning
- Tipo de issue
- Error
- Dificultad
- 5/5
- Tiempo estimado
- Más de una semana
- Estado de actividad
- Tranquilo
- Claridad
- Bien especificado
- Aptitud para principiantes
- 35/100