Skip to content

Append EOS to caption targets (train captions to stop) - #157

Merged
amazloumi merged 3 commits into
multimodal-videofrom
followup/vlm-caption-bos-eos
Jul 25, 2026
Merged

Append EOS to caption targets (train captions to stop)#157
amazloumi merged 3 commits into
multimodal-videofrom
followup/vlm-caption-bos-eos

Conversation

@amazloumi

Copy link
Copy Markdown
Member

Summary

  • _tokenize_and_mask appends the tokenizer's EOS to each caption (when defined), reserving its slot within max_text_len, so the last caption token is supervised to predict stop. No-op when the tokenizer has no EOS. Shared by the image + video caption paths.
  • Also resolves the single-token-caption all--100 edge case ([x][x, EOS]).
  • First-token supervision (unprompted): use a caption prompt — its last token predicts the first caption token (Qwen3 has no BOS, so nothing is prepended). Documented in the docstring.
  • Tests: EOS appended + predicted, single-token supervised, empty still masked.

Stacks on #155

Testing

  • ruff check / ruff format --check pass
  • pyright (0 errors)
  • pytest tests/unit/{test_vlm_dataset,test_video_dataset}.py — 53 passed

Closes #156

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates caption tokenization/label construction so captioning fine-tunes learn to emit a stop token by appending the tokenizer EOS (when available) within max_text_len, and extends unit tests to cover EOS supervision and prior edge cases (single-token and empty captions).

Changes:

  • Append eos_token_id (when defined) during _tokenize_and_mask so the last caption token is supervised to predict EOS.
  • Update/extend unit tests for both image (VLM) and video caption datasets to validate EOS supervision and edge cases.
  • Document the behavior in _tokenize_and_mask’s docstring and add a changelog entry.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
kempnerforge/data/vlm_dataset.py Appends EOS within max_text_len in _tokenize_and_mask and documents supervision semantics.
tests/unit/test_vlm_dataset.py Updates tests to assert EOS is appended/predicted and adds coverage for single-token + empty captions.
tests/unit/test_video_dataset.py Updates video dataset tests to match new EOS-append/label expectations.
CHANGELOG.md Records the EOS stop-target fix and notes prompt-related first-token supervision behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread kempnerforge/data/vlm_dataset.py
Base automatically changed from fix/vlm-label-alignment to multimodal-video July 24, 2026 20:57
@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
kempnerforge/config/vlm.py 100.00% <100.00%> (ø)
kempnerforge/data/vlm_dataset.py 99.11% <100.00%> (+0.02%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@camilobrownpinilla camilobrownpinilla left a comment

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.

lgtm

@amazloumi
amazloumi merged commit b7b7b85 into multimodal-video Jul 25, 2026
6 checks passed
@amazloumi
amazloumi deleted the followup/vlm-caption-bos-eos branch July 25, 2026 00:14
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.

4 participants