PR #4105 ·
all links pinned to f31f793
Four pages on this PR, one question each — this one: an order-dependent bug found while reviewing it.
When Energon builds a batch of packs, each token gets some extra per-token columns
alongside its id — for images, mm_token_type_ids marks which tokens are
picture and which are text. A conversation with no picture has no such column at all.
The packing code notices this and fills in zeros for the sources that lack it, so every
row lines up. It does that fill once per pack. A batch holds several
packs, and if the first one has no picture anywhere in it, the fill never
happens for the batch — and the column stops being a tensor.
Three things line up. Each is reasonable alone.
# 1. Packs are grouped by a key that ignores pictures, so a text-only pack # and a picture pack land in the same batch all the time. group_key = (self.fingerprint,) # generic_sft.py:249 # 2. The zero-fill template is built inside the per-pack loop, so it only # ever sees one pack. An all-text pack keeps no picture column. for pack in packs: templates = {...} # packing.py:107-121 # 3. The batch builder decides the column's type from row zero alone. if not values or not isinstance(values[0], Tensor): result[key] = values # stores the raw Python list continue # llm_message_utils.py:402 — the code just below this correctly # fills in zeros for missing rows. It never gets there.
The PR's author already reasoned about step 1 and wrote it down at generic_sft.py:239-242: grouping more finely "buys nothing", because the plain batch builder keeps these columns in per-message dictionaries and never stacks them. That is true of the older path. The packed path added by this PR does stack them — so the reason the coarse grouping was safe stops holding exactly where the new code runs.
Build the zero-fill template once for the whole batch instead of once per pack. Then a text-only pack gets its zeros no matter where it sits.
# before — inside the loop, sees one pack for pack in packs: templates = {...} # after — above the loop, sees every pack templates = { key: value for pack in packs # ← the whole batch for sample in pack.samples for message in sample.message_log for key, value in message.items() if key not in {"token_ids", "token_loss_mask"} and isinstance(value, torch.Tensor) } for pack in packs: ...
Replaces packing.py:107-121. The committed test at test_energon_packing.py:117 puts the text-only source first inside one pack, which the per-pack fill already handles; the missing case is a text-only pack first in the batch.
NonTensorData, so training
carries on with a per-token column that is no longer a tensor. There is no error message to
search for and no failed assertion — only whichever batches happened to start with a
text-only pack behave differently. What the model then does with that column was not
measured: there was no GPU available for this review, so the claim stops at "the column
stops being a tensor and nothing rejects it".