Repository navigation
fix: handle attention masks without a padding token - #1909
AaravMehta-07 wants to merge 1 commit into
Conversation
jlarson4
left a comment
There was a problem hiding this comment.
Just one comment, more detail about how the tokenizer system is connected that is relevant to this bug fix. Let me know if you have any questions
| def test_input_to_embed_without_pad_id_attends_every_token(): | ||
| bridge = TransformerBridge.boot_native(_cfg(device="cpu")) | ||
| bridge.eval() | ||
| bridge.tokenizer = TokenizersBackend( |
There was a problem hiding this comment.
The tokenizer only lacks a pad id here because assigning bridge.tokenizer after a tokenizer-less boot skips configure_tokenizer, using boot_native(cfg, tokenizer=...) would have set one. The setter treats any first assignment as the __init__ one, so this path also leaves to_tokens unable to tokenize and keeps a stale tokenizer_prepends_bos (a Qwen tokenizer attached this way silently loses its BOS). Could you look at fixing the first-assignment check in the tokenizer setter in bridge_core.py, so a tokenizer attached after boot gets the same setup as one passed at boot?
Description
get_attention_maskcallstokens.ne(tokenizer.pad_token_id)whenever a tokenizer is present. A tokenizer without a padding token haspad_token_id=None, so this raisesTypeErrorinstead of attending the supplied unpadded token tensor. This also reachesTransformerBridge.input_to_embedwith a left-padding tokenizer.Return the already-created all-ones mask when no padding ID exists. This does not mutate the tokenizer vocabulary or add model embeddings. Existing left/right padding and BOS-equals-pad handling are unchanged. The annotation now also reflects the existing
None-tokenizer branch.This is an independently reproduced bug; no existing issue is being closed.
Type Of Change
Validation
Executed on Python 3.12, CPU, through
uv runwith the local validation dependency overlay and normal project jaxtyping/beartype pytest options:Tensor.ne(None), as expected.python -m pytest -q -s tests/unit/utilities/test_tokenize_utils.py tests/unit/model_bridge/test_boot_native.py: 89 passed after the fix.python -m mypy .: no issues in 388 source files.make format,make check-format, andgit diff --check: passed. The upstream format recipe's literalisort format .reports a missing path; the separate actualisort --check-only .succeeds. Black skips untouched notebooks because its Jupyter extra is absent; Python formatting is checked.Tests cover left/right tokenizer settings and int32/int64 inputs; real padding/BOS controls; and a real tiny native bridge with a local WordLevel tokenizer, comparing mask, residuals and logits with an explicit all-ones mask. No model or HF load is mocked.
The full project suite, GPU/MPS checks and hosted CI have not been run locally.
Checklist
AI Assistance
Codex assisted with investigation, implementation, regression tests, local test execution, technical review and this description. The results above were executed locally. No human code review or maintainer approval is claimed.