Generation: honor suppress_tokens and mask multimodal placeholder tokens#412
Generation: honor suppress_tokens and mask multimodal placeholder tokens#412fdagostino wants to merge 2 commits into
Conversation
Multimodal placeholder tokens (e.g. <audio|> 258882 on gemma4_unified) were occasionally sampled as generated text because nothing masked their logits during sampling, and `suppress_tokens` declared in generation_config.json was never parsed. - GenerationConfigFile: parse `suppress_tokens` - SuppressedTokensProviding: models advertise token IDs that must never be sampled; model factories merge generation_config's suppress_tokens into the set after the model is created - SuppressTokensProcessor (port of transformers' SuppressTokensLogitsProcessor): masks those IDs to -inf before sampling, chained after the penalty processors in TokenIterator, SpeculativeTokenIterator and MTPSpeculativeTokenIterator - Gemma4 / Gemma4Unified: seed the set from the config placeholder IDs (image/audio/video soft tokens plus boi/eoi/boa/eoa markers) Only sampled tokens are affected; prompt tokens are untouched. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| public func mergeGenerationConfigSuppressedTokens( | ||
| _ generationConfig: GenerationConfigFile?, into model: any LanguageModel | ||
| ) { | ||
| guard let suppressTokens = generationConfig?.suppressTokens?.values else { return } |
There was a problem hiding this comment.
This makes generation_config.suppress_tokens a no-op for every model that does not adopt SuppressedTokensProviding; currently only the two Gemma 4 VLMs do...
| /// Returns `nil` when `tokenIds` is empty (nothing to suppress). | ||
| public init?(tokenIds: Set<Int>) { | ||
| guard !tokenIds.isEmpty else { return nil } | ||
| let sorted = tokenIds.sorted().map { UInt32($0) } |
There was a problem hiding this comment.
These IDs are not validated against the logits vocabulary. The tiny Gemma4Unified test configuration has vocab_size: 32 but inherits boi/boa defaults in the 255k range, so a text-only TokenIterator will pass out-of-range indices to putAlong. Please filter to 0..<model.vocabularySize (and handle negative IDs) before building the suppressor, with a generation regression test.
|
If going ahead with this PR, MTP draft proposals need the same suppression policy to avoid unnecessary verifier rejections and reduced speculative-decoding acceptance. |
…all models, suppress in MTP drafts - SuppressTokensProcessor drops negative IDs at init and IDs beyond logits.dim(-1) at process time, so tiny-vocab configs inheriting 255k-range placeholder defaults degrade gracefully instead of indexing out of range. - suppress_tokens from generation_config.json is now honored for every model: non-conforming models are tracked in a weak side table merged by the factories; makeLogitProcessor unions both sources. - MTP draftBlock receives the sampler wrapped in SuppressTokensSampler so drafts cannot propose suppressed tokens the verifier would reject. - 6 new regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Thanks for the careful review — all three points were valid and are addressed in cb9b074: (1) |
Problem
Two related gaps in the sampling path:
suppress_tokensfromgeneration_config.jsonwas silently ignored —GenerationConfigFilenever parsed the field, so checkpoints that declare tokens which must never be sampled got no masking at all.gemma4_unified(e.g.mlx-community/gemma-4-12B-it-4bit) we saw<audio|>(258882) show up in text-only replies — nasty for downstream consumers (imagine a TTS reading it aloud). The same symptom was reported independently against the same checkpoint family in another runtime: Gemma 4 12B Unified: multimodal placeholder tokens (e.g.<audio|>) leak into text-only generation jundot/omlx#1670. The shippedgeneration_config.jsononly suppresses 258883/258882 on some checkpoints, and even that was ignored (gap 1).Fix
Mirrors the
transformersdesign (SuppressTokensLogitsProcessor, driven bygeneration_config.suppress_tokens):GenerationConfigFile: parsesuppress_tokens.SuppressedTokensProvidingprotocol: models advertise token IDs that must never be sampled; the LLM/VLM model factories mergegeneration_config.json'ssuppress_tokensinto that set after the model is created.SuppressTokensProcessor(port of the transformers processor): masks those IDs to-infbefore sampling, chained after the parameter-derived penalty processor inTokenIterator,SpeculativeTokenIteratorandMTPSpeculativeTokenIteratorvia a smallChainedLogitProcessor.Gemma4/Gemma4Unified: seed the set from the config placeholder IDs (image/audio/video soft tokens plus boi/eoi/boa/eoa markers).Only sampled tokens are affected; prompt tokens are untouched, so multimodal inputs keep working unchanged.
Testing
Tests/MLXLMTests/SuppressTokensTests.swift(config parsing, processor masking/broadcasting, chaining, factory merge, Gemma 4 seeding).xcodebuild build-for-testing -scheme mlx-swift-lm-Package -destination 'platform=macOS'+xcrun xctest …/MLXLMTests.xctest(same as CI) pass on this branch.mlx-community/gemma-4-12B-it-4bit: the placeholder leakage disappears; generation output otherwise unchanged.Planned follow-up (separate PR)
App-driven per-generation suppression via a
suppressedTokensfield onGenerateParameters, merged into the samemakeLogitProcessorunion (and reaching the MTP draft sampler through the same path). This PR intentionally keeps checkpoint-level semantics only —generation_config.jsonand model-adopted sets — mirroring howtransformersscopessuppress_tokens; the per-generation field is for app decisions (e.g. suppressing a model's thinking-channel delimiters for a voice reply while another session of the same loaded model leaves them free).