Skip to content

Fix left-padding token selection in BioGptForSequenceClassification#46782

Merged
Rocketknight1 merged 1 commit into
huggingface:mainfrom
Sunt-ing:D-001-biogpt-leftpad-seqclass-pooling
Jun 22, 2026
Merged

Fix left-padding token selection in BioGptForSequenceClassification#46782
Rocketknight1 merged 1 commit into
huggingface:mainfrom
Sunt-ing:D-001-biogpt-leftpad-seqclass-pooling

Conversation

@Sunt-ing

Copy link
Copy Markdown
Contributor

What does this PR do?

BioGptForSequenceClassification is the last decoder classification head still using the legacy pooling index:

sequence_length = (torch.ne(input_ids, self.config.pad_token_id).sum(-1) - 1)
pooled_logits = logits[torch.arange(batch_size), sequence_length]

(num_non_pad - 1) equals the index of the last real token only when padding is on the right. With left padding the real tokens are shifted to the end, so this selects a token in the front of the sequence and the head pools the wrong position. The classification logits and loss are then silently wrong, with no error or warning. Right padding is unaffected.

#35911 moved every other ForSequenceClassification decoder (gpt2, llama, ...) onto a left/right-padding-safe selection, but biogpt was missed. This PR brings biogpt in line by using the same block as gpt2:

non_pad_mask = (input_ids != self.config.pad_token_id).to(logits.device, torch.int32)
token_indices = torch.arange(input_ids.shape[-1], device=logits.device, dtype=torch.int32)
last_non_pad_token = (token_indices * non_pad_mask).argmax(-1)

which takes the rightmost non-pad index and is correct for both padding sides. This is the full block from gpt2, so it also adds the same raise ValueError("Cannot handle batch sizes > 1 if no padding token is defined.") guard for pad_token_id is None with batch_size > 1 (previously biogpt silently pooled index -1); this matches every other decoder after #35911. The edit is in modular_biogpt.py, regenerated into modeling_biogpt.py with the modular converter.

biogpt is the only remaining model with this pattern (grep "torch.ne(input_ids, self.config.pad_token_id).sum" matches biogpt only).

Reproduction (CPU, fp32, no GPU)

The check stays inside a single forward to remove the position-embedding shift that left padding introduces: the head's pooled_logits should equal the per-position logits at the true last real token.

import torch
from transformers import BioGptConfig, BioGptForSequenceClassification, set_seed

torch.set_grad_enabled(False)
cfg = BioGptConfig(vocab_size=100, hidden_size=32, num_hidden_layers=2,
                   num_attention_heads=2, intermediate_size=64, num_labels=3, pad_token_id=0)
set_seed(0)
model = BioGptForSequenceClassification(cfg).eval()

set_seed(1)
real = torch.randint(1, cfg.vocab_size, (1, 5))          # 5 real tokens
input_ids = torch.cat([torch.zeros(1, 3, dtype=torch.long), real], dim=1)  # 3 left pads
mask = (input_ids != 0).long()

pooled = model(input_ids, attention_mask=mask).logits     # head's pooled logits
hidden = model.biogpt(input_ids, attention_mask=mask).last_hidden_state
per_pos = model.score(hidden)                             # same-forward per-position logits

last_real = input_ids.shape[1] - 1                        # index 7
print("d(pooled, last_real) =", (pooled - per_pos[:, last_real]).abs().max().item())

Before fix (selects index 4, the buggy num_non_pad - 1):

d(pooled, last_real) = 0.23832...

After fix (selects index 7, the real last token):

d(pooled, last_real) = 0.0

gpt2 already returns 0.0 here; right padding returns 0.0 for biogpt before and after.

Added regression test test_biogpt_sequence_classification_left_padding: it asserts pooled_logits equals the last-real-token logits from the same forward. It fails on the current code and passes with the fix. Full tests/models/biogpt/test_modeling_biogpt.py passes.

Code Agent Policy

  • I confirm that this is not a pure code agent PR.

Before submitting

  • This PR fixes a typo or improves the docs (you can dismiss the other checks if that's the case).
  • Did you read the contributor guideline, Pull Request section?
  • Did you write any new necessary tests?

Who can review?

cc @Rocketknight1

BioGptForSequenceClassification was the last decoder classification head
still using the legacy pooling index (input_ids != pad).sum(-1) - 1, which
is the last real token only under right padding. With left padding the real
tokens are shifted to the end, so the head pooled an earlier token and
returned silently wrong logits and loss. Use the same left/right-padding
safe selection as the other decoders (from huggingface#35911).

Signed-off-by: Ting Sun <[email protected]>
@github-actions

Copy link
Copy Markdown
Contributor

[For maintainers] Suggested jobs to run (before merge)

run-slow: biogpt

@github-actions

Copy link
Copy Markdown
Contributor

CI Dashboard: View test results in Grafana

@Rocketknight1 Rocketknight1 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, LGTM! Glad we can finally clean out the old code

@Rocketknight1
Rocketknight1 enabled auto-merge June 22, 2026 13:34
@Rocketknight1
Rocketknight1 added this pull request to the merge queue Jun 22, 2026
@HuggingFaceDocBuilderDev

Copy link
Copy Markdown

The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update.

Merged via the queue into huggingface:main with commit bfd3604 Jun 22, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants