Skip to content

[Fix] Include max_length and render options in GLM-5.3-Flash VL tokenize hash - #2145

Merged
jayhenry merged 1 commit into
InternLM:feat/glm53flash-f1-vl-datafrom
ShilohYu:fix/glm53-vl-tokenize-hash-configs
Oct 10, 2026
Merged

jayhenry merged 1 commit into
InternLM:feat/glm53flash-f1-vl-datafrom
ShilohYu:fix/glm53-vl-tokenize-hash-configs

Conversation

@ShilohYu

@ShilohYu ShilohYu commented Oct 9, 2026 •

Copy link
Copy Markdown
Collaborator

This fixes stale on-disk length caches for GLM-5.3-Flash VL training when max_length or the chat render options change. The cache path predicts per-sample num_tokens under the current config and persists them in a directory keyed by tokenize_fn.hash(), but _hash_str only encoded processor budgets and pack weights. Changing max_length, system_message, add_generation_prompt, enable_thinking, or reasoning_effort left the hash unchanged, so a later run reused old-config lengths: the packer budgets by old lengths while runtime truncates by new ones, and truncation landing inside a visual span degrades into silently packed fake samples.

Encode the five parameters in _hash_str, reading max_length from the constructor local since super().init stores it only afterwards. Add a regression test that classifies every init parameter as hash-affecting or not and asserts hash() changes for each one; each variant rebuilds its baseline so the shared processor cache cannot mask the assertion. Unclassified new parameters fail the test.

Stack placement: targets feat/glm53flash-f1-vl-data (#2109), so the fix can propagate into #2111. Note: existing on-disk length caches invalidate once on the next run, which is intended.

This PR also carries a small unrelated commit that repairs the lint gate on the base branch: 3de92e7 added a selector argument to tilelang_dsa_topk_indices but left tilelang_indexer_topk_from_ranges referencing it undeclared, failing ruff F821 and mypy for every stacked PR. The parameter is threaded through, with a dispatch test for both selector values.

Validation:

  • pytest tests/datasets/test_glm53_vl_tokenize_fn.py: 12 passed (11 existing + 1 new), transformers 5.17.0, real GLM-5.3-Flash-25B checkpoint.
  • Mutation check with the source fix stashed: the new test goes red on the first render-option variant (system_message); the pre-existing processor/pack invalidation test stays green.
  • Before/after probe: the five previously unhashed configs all equaled baseline before, pairwise-distinct after; the max_pixels control changed in both.
  • tests/ops/test_tilelang_dsa_topk_chunk.py::test_range_selector_dispatches: 2 passed.
  • Local pre-commit on all changed files: all hooks pass, including the previously failing ruff F821 and mypy.
  • No end-to-end training run or cache-directory reuse test is claimed; hash-level invalidation only.

…ize hash

Glm53VLTokenizeFunction persists cache-path length predictions in a
directory keyed by tokenize_fn.hash(), but _hash_str only encoded
processor budgets and pack weights. Changing max_length,
system_message, add_generation_prompt, enable_thinking, or
reasoning_effort left the hash unchanged, so a later run reused
old-config lengths: the packer budgets by old lengths while runtime
truncates by new ones, and truncation landing inside a visual span
degrades into silently packed fake samples.

Encode the five parameters in _hash_str, reading max_length from the
constructor local since super().__init__ stores it only afterwards.
Add a regression test that classifies every __init__ parameter as
hash-affecting or not and asserts hash() changes for each one; each
variant rebuilds its baseline so the shared processor cache cannot
mask the assertion.
@ShilohYu
ShilohYu force-pushed the fix/glm53-vl-tokenize-hash-configs branch 2 times, most recently from 4082f72 to 15cbeb1 Compare October 10, 2026 05:08
@jayhenry
jayhenry merged commit 753f1e1 into InternLM:feat/glm53flash-f1-vl-data Oct 10, 2026
0 of 4 checks passed
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.

2 participants