Skip to content

fix: zero left-padded tokens before the GDN recurrence - #223

Open
shiaho777 wants to merge 1 commit into
modelscope:mainfrom
shiaho777:fix/gdn-left-padding
Open

shiaho777 wants to merge 1 commit into
modelscope:mainfrom
shiaho777:fix/gdn-left-padding

Conversation

@shiaho777

Copy link
Copy Markdown
Contributor

GatedDeltaNet.forward received attention_mask and did not use it. A causal conv and the delta-rule update both carry state forward, so left-padding tokens change the real tokens that follow them. The loss mask removes those positions from the loss and does not clear the state.

Padding keys are the positions masked for every query, the same reduction gpt_model uses (attention_mask.all(dim=(1, 2))). Those hidden states are multiplied by zero before the projection. A mask whose shape is not [batch, seq] is left unchanged. Packed sequences keep using cu_seqlens and are not masked here.

Checked with python3 -m pytest tests/test_gdn_padding_keep.py. A [1, 1, 3, 3] mask with the first key padded produces keep [0, 1, 1]. A mask of the wrong length returns None. flake8 is clean.

GatedDeltaNet.forward received attention_mask and did not use it. A causal conv and the delta-rule update both carry state forward, so left-padding tokens change the real tokens that follow them. The loss mask removes those positions from the loss and does not clear the state.

Padding keys are the positions masked for every query, the same reduction gpt_model uses (attention_mask.all(dim=(1, 2))). Those hidden states are multiplied by zero before the projection. A mask whose shape is not [batch, seq] is left unchanged. Packed sequences keep using cu_seqlens and are not masked here.

Checked with python3 -m pytest tests/test_gdn_padding_keep.py. A [1, 1, 3, 3] mask with the first key padded produces keep [0, 1, 1]. A mask of the wrong length returns None.
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