System Info

• transformers: main (7ea2320c76), also present in every release since v4.49
• Code-level bug, weight-independent — reproducible on CPU, no GPU or pretrained weights needed

Who can help?

@zucchini-nlp

Description

GitForCausalLM.forward shifts the labels manually and then hands the result to self.loss_function, which shifts them a second time. The model is trained to predict token t + 2 from position t instead of t + 1.

src/transformers/models/git/modeling_git.py:1081-1092:

if labels is not None:
    # we are doing next-token prediction; shift prediction scores and input ids by one
    num_image_tokens = self.git.encoder.layer[0].attention.self.image_patch_tokens
    shifted_logits = logits[:, num_image_tokens:-1, :].contiguous()
    labels = labels[:, 1:].contiguous()              # shift #1
    loss = self.loss_function(
        shifted_logits.view(-1, self.config.vocab_size),
        labels.view(-1),                             # binds positionally to `labels`
        vocab_size=self.config.vocab_size,
        **kwargs,
    )

GitForCausalLM resolves loss_type to "ForCausalLM", so self.loss_function is ForCausalLMLoss. Its signature is (logits, labels, vocab_size, num_items_in_batch, ignore_index, shift_labels), so the already-shifted tensor binds to labels and shift_labels stays None. src/transformers/loss/loss_utils.py:61-64 then does:

if shift_labels is None:
    labels = nn.functional.pad(labels, (0, 1), value=ignore_index)
    shift_labels = labels[..., 1:].contiguous()      # shift #2

Two things follow from this. First, the targets are off by one extra position. Second — and this is the part that does not show up in the loss value alone — the manual shift flattens the labels to 1-D before the call, so the internal pad-and-slice operates on the flattened tensor and the shape stays consistent (N → N+1 → N). No error is raised, and each batch row's final target is silently pulled in from the next row of the batch.

Regression

This was correct until #35875 ("Fix model kwargs", 28f73bc307), which mechanically swapped the loss implementation but left the manual shift in place:

-            loss_fct = CrossEntropyLoss()
-            loss = loss_fct(shifted_logits.view(-1, self.config.vocab_size), labels.view(-1))
+            loss = self.loss_function(
+                shifted_logits.view(-1, self.config.vocab_size),
+                labels.view(-1),

CrossEntropyLoss does not shift, ForCausalLMLoss does. This is the same regression mechanism that was fixed for Moonshine in #46784.

I checked the rest of what #35875 touched. On current main only three call sites still combine a manual shift with self.loss_function: csm is correct (it passes labels=None, shift_labels=shift_labels), moshi has the same bug but is already covered by the open #36928, and git is the remaining unfixed one.

Reproduction

CPU, no pretrained weights:

import torch
from torch.nn import CrossEntropyLoss

from transformers import GitConfig, GitForCausalLM
from transformers.models.git.configuration_git import GitVisionConfig

torch.manual_seed(0)
vision_config = GitVisionConfig(
    hidden_size=32, num_hidden_layers=2, num_attention_heads=4,
    intermediate_size=37, image_size=32, patch_size=16,
)
config = GitConfig(
    vision_config=vision_config.to_dict(), vocab_size=99, hidden_size=32,
    num_hidden_layers=2, num_attention_heads=4, intermediate_size=37,
)
model = GitForCausalLM(config).eval()

input_ids = torch.randint(3, config.vocab_size, (2, 7))
pixel_values = torch.randn(2, 3, 32, 32)
labels = input_ids.clone()

with torch.no_grad():
    out = model(input_ids=input_ids, pixel_values=pixel_values, labels=labels)

num_image_tokens = model.git.encoder.layer[0].attention.self.image_patch_tokens
shifted_logits = out.logits[:, num_image_tokens:-1, :]
V = config.vocab_size

aligned = CrossEntropyLoss()(shifted_logits.reshape(-1, V), labels[:, 1:].reshape(-1))

# what the code actually computes: a second shift, applied to the flattened targets
flat = labels[:, 1:].reshape(-1)
leaked = torch.nn.functional.pad(flat, (0, 1), value=-100)[1:]
double_shifted = CrossEntropyLoss()(shifted_logits.reshape(-1, V), leaked)

print("model loss            :", out.loss.item())
print("aligned CE (expected) :", aligned.item())
print("double-shifted CE     :", double_shifted.item())
print("matches aligned       :", torch.allclose(out.loss, aligned))
print("matches double-shift  :", torch.allclose(out.loss, double_shifted))
print("row 0 correct targets :", labels[0, 1:].tolist())
print("row 0 actual targets  :", leaked[:6].tolist())
print("row 1 labels          :", labels[1, 1:].tolist())

Output on main:

model loss            : 4.584834575653076
aligned CE (expected) : 4.646134853363037
double-shifted CE     : 4.584834575653076
matches aligned       : False
matches double-shift  : True
row 0 correct targets : [28, 71, 8, 58, 61, 15]
row 0 actual targets  : [71, 8, 58, 61, 15, 19]
row 1 labels          : [19, 42, 30, 58, 94, 67]

The loss matches the double-shifted computation exactly, and row 0's last target (19) is row 1's first label.

Expected behavior

The target for position t should be labels[t + 1], and targets must not cross batch-row boundaries.

Suggested fix

Unlike Moonshine, GIT cannot simply drop the manual shift: logits contains num_image_tokens leading image positions that have to be sliced off regardless. Passing shift_labels explicitly suppresses the internal shift, which is the convention already used by csm (modeling_csm.py:619-621):

             shifted_logits = logits[:, num_image_tokens:-1, :].contiguous()
-            labels = labels[:, 1:].contiguous()
+            shift_labels = labels[:, 1:].contiguous()
             loss = self.loss_function(
-                shifted_logits.view(-1, self.config.vocab_size),
-                labels.view(-1),
+                logits=shifted_logits,
+                labels=None,
                 vocab_size=self.config.vocab_size,
+                shift_labels=shift_labels,
                 **kwargs,
             )

Keeping the tensors 2-D also removes the cross-row leak, since ForCausalLMLoss flattens them itself. With this applied, the repro above prints 4.646135 for both the model loss and the aligned CE. I have the fix and a regression test ready (the full tests/models/git suite passes: 190 passed, 297 skipped) and am happy to open a PR if you agree with the direction.

Disclosure: this investigation and the proposed patch were prepared with AI assistance, and reviewed and verified by me.