Skip to content

docs(attention): clarify make_causal_mask and make_attention_mask docstrings (#1520) - #5563

Open
Ultron09 wants to merge 1 commit into
google:mainfrom
Ultron09:doc/make-causal-mask-docs
Open

docs(attention): clarify make_causal_mask and make_attention_mask docstrings (#1520)#5563
Ultron09 wants to merge 1 commit into
google:mainfrom
Ultron09:doc/make-causal-mask-docs

Conversation

@Ultron09

Copy link
Copy Markdown

What does this PR do?

Fixes #1520

The make_causal_mask and make_attention_mask functions generate masks of shape [batch..., 1, len_q, len_kv] for multi-head attention. However, users frequently pass embedded feature arrays ([batch..., len, num_features]) instead of sequence arrays ([batch..., len]), causing the sequence length to erroneously bind to num_features.

This PR:

  1. Expands docstrings in both flax.linen.attention and flax.nnx.nn.attention with explicit shape explanations, notes on un-embedded slices vs embedded feature inputs, and how the singleton dimension broadcasts across attention heads.
  2. Adds executable doctests and examples to make_attention_mask and make_causal_mask for both token IDs and embedded inputs.

Checklist

  • This PR fixes a minor issue (e.g.: typo or small bug) or improves the docs (you can dismiss the other checks if that's the case).
  • This change is discussed in a Github issue/discussion.
  • The documentation and docstrings adhere to the documentation guidelines.
  • Tested with doctest (passed all 90 attempted tests across Linen & NNX attention modules).

cc @marcvanzee @vfdev-5 for review.

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.

Improve documentation of make_causal_mask

2 participants