fix(metrics): mask ignored labels in padding-free seq_acc - #10049
Open
MaxFreedomPollard wants to merge 1 commit into
Open
fix(metrics): mask ignored labels in padding-free seq_acc#10049MaxFreedomPollard wants to merge 1 commit into
MaxFreedomPollard wants to merge 1 commit into
Conversation
`compute_acc` in swift/metrics/acc.py scores each packed sequence by slicing `preds` and `labels` with `cu_seqlens`, but that branch never applies the `labels != -100` mask that the padded branch right below it applies. Prompt tokens carry the label -100 and predictions are token ids, so every slice that contains a prompt compares -100 against a token id and `np.all` returns False. With `--padding_free` and `--acc_strategy seq`, seq_acc is therefore reported as 0 for every sequence, including sequences the model answered perfectly. Slice `masks` alongside `preds` and `labels` so the padding-free branch scores the same positions as the padded one. Added tests/utils/test_acc_metrics.py, which checks a packed batch of two sequences and asserts the padding-free result equals the result for the same data as an ordinary padded batch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR type
PR information
seq_accis always 0 when training with--padding_free true --acc_strategy seq, even for sequences the model predicts perfectly.compute_accinswift/metrics/acc.pybuildsmasks = labels != -100and then splits in two. The padded branch scoresnp.all(preds[i, m] == labels[i, m]), so it only looks at supervised positions. The padding-free branch, whichSeq2SeqTrainer.compute_lossreaches by passingcu_seqlens, scoresnp.all(preds[0, start:end] == labels[0, start:end])and never touchesmasks. Prompt tokens carry the label -100 andpredsholds token ids, so any packed sequence with a prompt in front of it compares -100 against a token id,np.allis False, and the metric reports 0.The fix slices
masksalongsidepredsandlabels, so the padding-free branch scores the same positions the padded branch already scores.acc_strategy=tokenand the paddedseqpath do not reach this branch and are unchanged.tests/utils/test_acc_metrics.pypacks two sequences of two prompt tokens plus two response tokens, the first answered correctly and the second not. It asserts the result is[True, False], and that it equals the result for the same data laid out as an ordinary padded batch.Experiment results
Both new tests fail on main and pass with this change.
python -m pytest tests/utils/test_acc_metrics.py -qon unmodified main:2 failed, the assertion diff being- [False, False]against+ [True, False].python -m pytest tests/utils/test_acc_metrics.py -qwith this change:2 passed.python -m pytest tests/utils -q:1 failed, 89 passed, 22 skipped. The single failure istest_opsd_teacher_images.py::test_real_qwen2_vl_template_handles_fewer_teacher_images_than_tags, which fails identically on unmodified main in my environment becauseqwen_vl_utilsis not installed.pre-commit run --files swift/metrics/acc.py tests/utils/test_acc_metrics.py: flake8, isort and yapf all pass.