huggingface / huggingface/nanoVLM
Are position IDs handled correctly for packed sequences during training?
- Dominant language
- Python
- Stars
- 5k
- Forks
- 510
- PR merge metrics
- No merged PRs in 30d
Description
Hi, thanks for the great work! I have a question about how position IDs are computed in LanguageModel.forward().
In forward(), position IDs are generated as a simple contiguous range:
`current_position_ids = torch.arange(start_pos, start_pos + T_curr, device=x.device) \
.unsqueeze(0).expand(B, -1)`
My concern
ConstantLengthDataset packs multiple independent samples into a single fixed-length sequence via _pack_one_group:
`Packed sequence: [SampleA_tok0, ..., SampleA_tok99, PAD, SampleB_tok0, ..., SampleB_tok199, PAD, ...]
Position IDs: [ 0, ..., 99, 100, 101, ..., 300, 301, ...]`
This means Sample B's first token receives position ID 101 instead of 0. The RoPE encoding applied to Sample B would correspond to positions it would never see during inference, where every sequence starts from position 0.
Shouldn't each sample within the packed sequence have its position IDs reset to start from 0? Something like:
`Expected IDs: [0, ..., 99, 100, 0, ..., 199, 200, ...]
├─ Sample A ─┤ ├─ Sample B ──┤
`
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at LanguageModel.forward() and ConstantLengthDataset._pack_one_group, then trace how packed samples and RoPE position IDs are handled during training. Compare the position IDs for a packed batch with equivalent samples processed separately, and verify whether masking or other logic changes the expected behavior. Done means the question has a verified explanation and, if needed, a focused regression test or implementation decision.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- machine-learning
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100