Skip to content

Exclude trailing padding token from the fine-tuning loss - #1697

Open
yentur wants to merge 1 commit into
ml-explore:mainfrom
yentur:fix/loss-trailing-pad-token
Open

Exclude trailing padding token from the fine-tuning loss#1697
yentur wants to merge 1 commit into
ml-explore:mainfrom
yentur:fix/loss-trailing-pad-token

Conversation

@yentur

@yentur yentur commented Aug 9, 2026

Copy link
Copy Markdown

Problem

default_loss counts one padding position per sequence. iterate_batches right pads
each row with zeros and reports lengths[:, 1] = L, the number of real tokens, but the
mask selects L - offset + 1 targets and the extra one is the pad value 0.

So every fine-tuned sequence is trained to predict token id 0 after its last token, and
ntoks is one too high. With --mask-prompt that padded position is a large share of
the signal: a six token prompt with a three token answer counts four targets per row
instead of three.

Root cause

mlx_lm/tuner/trainer.py:92

steps = mx.arange(1, targets.shape[1] + 1)
mask = mx.logical_and(steps >= lengths[:, 0:1], steps <= lengths[:, 1:])

targets[k] == batch[k + 1], so steps[k] is the index of targets[k] in the
un-shifted row. Real tokens occupy 0..L-1, so the last valid target index is L - 1
and steps <= L reaches one past the end.

Fix

steps <= lengths[:, 1:] becomes steps < lengths[:, 1:].

step == L is in range only when targets.shape[1] >= L, which happens only if the row
was padded, so this cannot drop a real target. Rows truncated to max_seq_length have
no padding column and come out unchanged. Reported train and validation loss will move
slightly, since ntoks is the denominator.

Verification

Four rows, six token prompt and three token answer, driven through the real
iterate_batches and default_loss. Before:

ntoks counted by default_loss : 16
real completion targets       : 12

loss with padding filled by 0  : 4.781232
loss with padding filled by 77 : 4.754244

After:

ntoks counted by default_loss : 12
real completion targets       : 12

loss with padding filled by 0  : 4.745682
loss with padding filled by 77 : 4.745682

The second pair is the invariant worth holding: changing only the bytes in the padding
region must not change the loss.

The two new tests fail on main,

$ python -m pytest tests/test_finetune.py -k TestDefaultLoss -q
E       AssertionError: 8 != 6
E       AssertionError: 3.734814167022705 != 3.6676673889160156
2 failed, 15 deselected

and pass with the fix:

2 passed, 15 deselected in 6.35s

test_finetune.py, test_tuner_trainer.py, test_tuner_utils.py and test_losses.py
give 29 passed. test_datsets.py::TestDatasets::test_hf fails on an unmodified checkout
too, huggingface_hub rejects the bare billsum name.

A 20 iteration LoRA run on mlx-community/Qwen2.5-0.5B-Instruct-4bit with
--mask-prompt trains normally, val 2.249 to 0.585.

default_loss masks targets with `steps <= lengths[:, 1]`, but `steps[i]` is
the index of `targets[i]` in the un-shifted row, so the last real target sits
at index `length - 1` and the `<=` bound reaches one index past the end, into
the zeros iterate_batches padded the row with.

Every sequence is therefore trained to predict token id 0 after its last
token, and ntoks is one too high. With completion only training that is a
large share of the loss: a six token prompt with a three token answer counts
four targets per row instead of three. The dropped position is in range only
when the row was padded, so no real target can be lost.
@yentur
yentur force-pushed the fix/loss-trailing-pad-token branch from 013560e to 7a02faa Compare August 9, 2026 10:59
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.

1 participant