Skip to content

Fix MiniCPM-o bounds offset after left padding - #10202

Open
zm249 wants to merge 1 commit into
modelscope:mainfrom
zm249:fix/minicpmo45-bound-offset
Open

zm249 wants to merge 1 commit into
modelscope:mainfrom
zm249:fix/minicpmo45-bound-offset

Conversation

@zm249

@zm249 zm249 commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

PR type

  • Bug Fix
  • New Feature
  • Document Updates
  • More Models or Datasets Support

PR information

MiniCPM-o image and audio bounds are computed before batch collation.

During batched inference, shorter samples are left-padded by the base
collator, but image_bound and audio_bounds were not shifted by the
corresponding padding length. This causes multimodal bounds to become
misaligned for shorter samples when batch_size > 1.

This PR shifts image_bound and audio_bounds by each sample's actual
left-padding length after the base collator determines the padded sequence
length.

The adjustment is applied only when padding-free mode is disabled and the
effective padding side is left.

Experiment results

  • Batch size 1: behavior is unchanged.
  • Batch size 4: bounds of shorter samples receive the correct padding offset.
  • The longest sample remains unchanged because its padding length is zero.
  • Local collator checks passed.
  • Existing related tests passed.

res.update(Template._data_collator(self, batch, padding_to=padding_to))

# image_bound and audio_bounds are computed before batching. Keep them aligned with input_ids when the
# base collator left-pads shorter samples. In inference, the base collator always uses left padding;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for solving this issue! I think other minicpm templates share the same issue, this function can be used as a common helper all of them

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.

2 participants