Skip to content

fix(torch): sum masked token losses in MaskedSoftmaxCELoss - #2717

Closed
chessing234 wants to merge 1 commit into
d2l-ai:masterfrom
chessing234:fix/masked-softmax-ce-loss-mean-to-sum
Closed

chessing234 wants to merge 1 commit into
d2l-ai:masterfrom
chessing234:fix/masked-softmax-ce-loss-mean-to-sum

Conversation

@chessing234

Copy link
Copy Markdown

Summary

MaskedSoftmaxCELoss.forward used .mean(dim=1) over the sequence dimension, dividing each sample's loss by num_steps even though padding positions already have weight 0.

Root cause

train_seq2seq accumulates l.sum() and reports metric[0] / Y_valid_len.sum(). That expects per-sample loss to be the sum over valid tokens. With .mean, loss is scaled down by num_steps, so the final metric is off by roughly num_steps / valid_len and training under-penalizes EOS (sequences stay long).

Fix

Use .sum(dim=1) so only valid-token cross-entropy contributes; downstream normalization by total valid tokens is then correct.

Fixes #2076

Made with Cursor

Co-authored-by: Cursor <cursoragent@cursor.com>
@chessing234 chessing234 closed this by deleting the head repository Aug 2, 2026
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.

MaskedSoftmaxCELoss code wrong

1 participant