Repository navigation
Add native CogVLM2-Video support - #48870
trueoneplusone wants to merge 1 commit into
Conversation
|
[For maintainers] Suggested jobs to run (before merge) run-slow: auto, cogvlm2 |
CI recapDashboard: View test results in Grafana
|
zucchini-nlp
left a comment
zucchini-nlp
left a comment
There was a problem hiding this comment.
We don't need the model in core library. Each model bears high maintenace cost and CogVLM is just not worth it!
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Model linter — first pass
transformers-mlinter found 19 item(s) in the model files this PR touches. These are structural conventions a maintainer would otherwise flag by hand.
This is automated and advisory — it does not block merging.
| rule | count | what it checks |
|---|---|---|
TRF057 |
5 | Public model, config, output and processor classes and their public methods must carry @auto_docstring. |
TRF012 |
3 | _init_weights must use init primitives, not in-place operations on module weights. |
TRF010 |
2 | Direct config definitions must use @strict(accept_kwargs=True). |
TRF026 |
1 | A module whose forward only delegates to its single submodule adds nothing; inline it. |
TRF058 |
1 | Buffers must be declared as nn.Buffer attributes, not registered with register_buffer(). |
TRF050 |
1 | One rotary module per model: attention classes must not instantiate their own rotary embedding. |
TRF043 |
1 | Attention classes must not declare position_ids in their forward signature; it flows via **kwargs. |
TRF018 |
1 | _init_weights overrides should call super()._init_weights(module), except modular-file sentinels. |
TRF056 |
1 | forward() must not materialize tensors with .item() or .tolist(). |
TRF015 |
1 | Models with non-empty _tied_weights_keys must have tie_word_embeddings in their Config. |
TRF048 |
1 | _tied_weights_keys must be a dict mapping target to source, not the pre-v5 list form. |
TRF019 |
1 | A processor TypedDict must not define _defaults; ship them on the hub in processor_config.json. |
7 item(s) outside this diff
src/transformers/models/cogvlm2/modular_cogvlm2.py:437TRF012 — TRF012:_init_weights(self, module)uses an in-place operation on a module's weight. Please use theinitfunctions primitives instead, usually imported asfrom ... import initialization as initsrc/transformers/models/cogvlm2/modular_cogvlm2.py:439TRF012 — TRF012:_init_weights(self, module)uses an in-place operation on a module's weight. Please use theinitfunctions primitives instead, usually imported asfrom ... import initialization as initsrc/transformers/models/cogvlm2/modular_cogvlm2.py:441TRF012 — TRF012:_init_weights(self, module)uses an in-place operation on a module's weight. Please use theinitfunctions primitives instead, usually imported asfrom ... import initialization as initsrc/transformers/models/cogvlm2/modular_cogvlm2.py:466TRF057 — TRF057:CogVLM2Model.get_video_featuresin modeling_cogvlm2.py has no @auto_docstring. No parent method it could inherit the decorator from declares it. Add @auto_docstring as the innermost decorator here.src/transformers/models/cogvlm2/modular_cogvlm2.py:503TRF056 — TRF056:.item()insideforwardmaterializes a tensor on the host. It breaks the dynamo graph and failstorch.export. Keep the value as a tensor.src/transformers/models/cogvlm2/modular_cogvlm2.py:549TRF015 — TRF015: CogVLM2ForConditionalGeneration defines _tied_weights_keys but configuration_cogvlm2.py maps to CogVLM2Config, which does not declare tie_word_embeddings. Add a top-level 'tie_word_embeddings: bool = ...' field to CogVLM2Config.src/transformers/models/cogvlm2/modular_cogvlm2.py:550TRF048 — TRF048: CogVLM2ForConditionalGeneration declares _tied_weights_keys as a list. The v5 form is a dict mapping target to source, e.g. {"lm_head.weight": "model.embed_tokens.weight"}.
| return self.weight * hidden_states.to(input_dtype) | ||
|
|
||
|
|
||
| class CogVLM2VisionConfig(PreTrainedConfig): |
There was a problem hiding this comment.
TRF010 — TRF010: CogVLM2VisionConfig directly inherits PreTrainedConfig but is missing @strict.
Why this matters
Without it a new config misses the repo's runtime type-validation contract and drifts from the dataclass-based config standard.
+@strict(accept_kwargs=True)
class AcmeConfig(PreTrainedConfig):
...Suppress with # trf-ignore: TRF010 if intentional.
| return self.weight * hidden_states.to(input_dtype) | ||
|
|
||
|
|
||
| class CogVLM2VisionConfig(PreTrainedConfig): |
There was a problem hiding this comment.
TRF057 — TRF057: CogVLM2VisionConfig in configuration_cogvlm2.py has no @auto_docstring. No parent class it could inherit the decorator from declares it. Add @auto_docstring as the innermost decorator here.
Why this matters
Without it a class ships with no intro and no parameter documentation, and a method with no argument documentation, no Returns section and no usage example -- all of which then have to be hand-written per model instead of coming from auto_docstring.py.
+@auto_docstring
@dataclass
class AcmeModelOutputWithPast(ModelOutput):
logits: torch.FloatTensor | None = None
+@auto_docstring
class AcmeForConditionalGeneration(AcmePreTrainedModel):
+ @auto_docstring
def forward(self, input_ids, pixel_values=None, **kwargs):
...Suppress with # trf-ignore: TRF057 if intentional.
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
| class CogVLM2Config(PreTrainedConfig): |
There was a problem hiding this comment.
TRF010 — TRF010: CogVLM2Config directly inherits PreTrainedConfig but is missing @strict.
Why this matters
Without it a new config misses the repo's runtime type-validation contract and drifts from the dataclass-based config standard.
+@strict(accept_kwargs=True)
class AcmeConfig(PreTrainedConfig):
...Suppress with # trf-ignore: TRF010 if intentional.
| super().__init__(**kwargs) | ||
|
|
||
|
|
||
| class CogVLM2Config(PreTrainedConfig): |
There was a problem hiding this comment.
TRF057 — TRF057: CogVLM2Config in configuration_cogvlm2.py has no @auto_docstring. No parent class it could inherit the decorator from declares it. Add @auto_docstring as the innermost decorator here.
Why this matters
Without it a class ships with no intro and no parameter documentation, and a method with no argument documentation, no Returns section and no usage example -- all of which then have to be hand-written per model instead of coming from auto_docstring.py.
+@auto_docstring
@dataclass
class AcmeModelOutputWithPast(ModelOutput):
logits: torch.FloatTensor | None = None
+@auto_docstring
class AcmeForConditionalGeneration(AcmePreTrainedModel):
+ @auto_docstring
def forward(self, input_ids, pixel_values=None, **kwargs):
...Suppress with # trf-ignore: TRF057 if intentional.
| return self.down_proj(self.act_fn(self.gate_proj(hidden_states)) * self.up_proj(hidden_states)) | ||
|
|
||
|
|
||
| class CogVLM2VisionExpertMLP(nn.Module): |
There was a problem hiding this comment.
TRF026 — TRF026: CogVLM2VisionExpertMLP only forwards to self.language_mlp. Drop the wrapper and use the inner module where it is called.
Why this matters
The wrapper adds a level to every weight name, to _no_split_modules, parallelism plans and every conversion mapping, while computing nothing -- and readers have to open one more class to find that out. PreTrainedModel subclasses are exempt: they exist for from_pretrained and the auto classes even when forward only delegates.
-class AcmeAtomTransformer(nn.Module):
- def __init__(self, config):
- super().__init__()
- self.encoder = AcmeEncoder(config)
-
- def forward(self, hidden_states, **kwargs):
- return self.encoder(hidden_states, **kwargs)
-
class AcmeModel(AcmePreTrainedModel):
def __init__(self, config):
super().__init__(config)
- self.atom_transformer = AcmeAtomTransformer(config)
+ self.encoder = AcmeEncoder(config)Suppress with # trf-ignore: TRF026 if intentional.
| self.language_expert_dense = nn.Linear(self.hidden_size, self.hidden_size, bias=False) | ||
| self.rotary_emb = CogVLM2RotaryEmbedding(config) | ||
|
|
||
| def forward( |
There was a problem hiding this comment.
TRF043 — TRF043: CogVLM2Attention.forward declares position_ids in its signature. Attention modules receive position_ids through **kwargs so padding-free flash-attention can consume it; take position_embeddings and **kwargs instead.
Why this matters
position_ids is consumed downstream by flash-attention padding-free training and must flow through **kwargs. Naming it in the signature swallows it before the attention interface can read it; the llama standard passes position_embeddings plus **kwargs.
class AcmeAttention(nn.Module):
def forward(
self,
hidden_states,
position_embeddings,
attention_mask=None,
- position_ids=None,
**kwargs: Unpack[TransformersKwargs],
):Suppress with # trf-ignore: TRF043 if intentional.
| return residual + hidden_states | ||
|
|
||
|
|
||
| class CogVLM2PreTrainedModel(PreTrainedModel): |
There was a problem hiding this comment.
TRF057 — TRF057: CogVLM2PreTrainedModel in modeling_cogvlm2.py has no @auto_docstring. No parent class it could inherit the decorator from declares it. Add @auto_docstring as the innermost decorator here.
Why this matters
Without it a class ships with no intro and no parameter documentation, and a method with no argument documentation, no Returns section and no usage example -- all of which then have to be hand-written per model instead of coming from auto_docstring.py.
+@auto_docstring
@dataclass
class AcmeModelOutputWithPast(ModelOutput):
logits: torch.FloatTensor | None = None
+@auto_docstring
class AcmeForConditionalGeneration(AcmePreTrainedModel):
+ @auto_docstring
def forward(self, input_ids, pixel_values=None, **kwargs):
...Suppress with # trf-ignore: TRF057 if intentional.
| } | ||
| _keys_to_ignore_on_load_unexpected = [r"model\.layers\.\d+\.self_attn\.rotary_emb\.inv_freq"] | ||
|
|
||
| def _init_weights(self, module): |
There was a problem hiding this comment.
TRF018 — TRF018: _init_weights of CogVLM2PreTrainedModel does not call super()._init_weights(...). If this is intentional, suppress with # trf-ignore: TRF018 on the line above the method.
Why this matters
The base _init_weights covers the standard module types (Linear, Embedding, LayerNorm, RotaryEmbedding, ...). Skipping it leaves every submodule the override misses uninitialized -- which passes tests and surfaces much later as a subtle weight-init bug (cf. #45597).
from ... import initialization as init
def _init_weights(self, module):
+ super()._init_weights(module)
if isinstance(module, AcmeCustomLayer):
- module.gate.data.zero_()
+ init.zeros_(module.gate)Suppress with # trf-ignore: TRF018 if intentional.
|
|
||
|
|
||
| class CogVLM2ProcessorKwargs(ProcessingKwargs, total=False): | ||
| _defaults = { |
There was a problem hiding this comment.
TRF019 — TRF019: CogVLM2ProcessorKwargs sets _defaults in code. Move processor defaults to processor_config.json instead.
Why this matters
Hardcoded _defaults scatter processor configuration across Python source, are awkward to override from config, and bloat the code. In processor_config.json on the hub they travel with the checkpoint and can be updated without a code change.
class Gemma4ProcessorKwargs(ProcessingKwargs, total=False):
- _defaults = {
- "text_kwargs": {"padding": False},
- "images_kwargs": {"return_tensors": "pt"},
- }
images_kwargs: Gemma4ImageProcessorKwargsSuppress with # trf-ignore: TRF019 if intentional.
| prompt += f"Question: {old_query} {answer_format} {response}\n" | ||
| return prompt + f"Question: {query} {answer_format}" | ||
|
|
||
| def __call__( |
There was a problem hiding this comment.
TRF057 — TRF057: CogVLM2Processor.__call__ is a public method without @auto_docstring, so its arguments, return value and usage example are left undocumented. Add @auto_docstring as the innermost decorator.
Why this matters
Without it a class ships with no intro and no parameter documentation, and a method with no argument documentation, no Returns section and no usage example -- all of which then have to be hand-written per model instead of coming from auto_docstring.py.
+@auto_docstring
@dataclass
class AcmeModelOutputWithPast(ModelOutput):
logits: torch.FloatTensor | None = None
+@auto_docstring
class AcmeForConditionalGeneration(AcmePreTrainedModel):
+ @auto_docstring
def forward(self, input_ids, pixel_values=None, **kwargs):
...Suppress with # trf-ignore: TRF057 if intentional.
|
Thanks for the CogVLM2-Video port! As @zucchini-nlp explained (#48870 (review)), we're not adding CogVLM2 to the core library given its maintenance cost; sharing it on the Hub with Unless the quality / simplicity of the code is majorely improved! |
What does this PR do?
Adds native Transformers support for CogVLM2-Video, addressing #47713.
The released zai-org/cogvlm2-video-llama3-chat checkpoint currently relies on trust_remote_code=True, and its legacy generation/cache implementation is not compatible with newer Transformers cache internals.
This draft implements the model as a first-class Transformers architecture:
Validation performed so far
Local focused regression tests:
The released checkpoint naming is intentionally preserved, including model.layers..mlp.language_mlp., self_attn.language_expert_, model.vision.patch_embedding., model.vision.transformer.layers., model.vision.linear_proj., model.vision.conv.*, and model.vision.{boi,eoi}.
Remaining work before marking ready
Legacy checkpoint metadata note
The current zai-org/cogvlm2-video-llama3-chat/config.json contains auto_map but does not contain a model_type field. Current AutoConfig.from_pretrained() therefore does not treat it as local/native code. After this integration lands, the Hub checkpoint needs model_type: cogvlm2 plus native processor metadata, so users can load it without trust_remote_code=True.
This is intentionally a draft until checkpoint-level parity and full CI are complete.
🤖 mlinter review state
d727e883393cb1aa