[https://nvbugs/6293015][fix] Add a delegating `@property def vocab_size_padded(self) -> int: return… - #15219
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughKimiK25ForConditionalGeneration now exposes a ChangesVLM wrapper vocabulary size property
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Hi @NVIDIA/trt-llm-torch-models-devs please help to review this PR, only a small fix for a NV bug. |
|
/bot run |
|
PR_Github #53426 [ run ] triggered by Bot. Commit: |
|
PR_Github #53426 [ run ] completed with state
|
|
/bot run |
1 similar comment
|
/bot run |
|
PR_Github #53511 [ run ] triggered by Bot. Commit: |
|
PR_Github #53511 [ run ] completed with state
|
|
/bot run |
|
PR_Github #53535 [ run ] triggered by Bot. Commit: |
|
PR_Github #53535 [ run ] completed with state
|
51a7963 to
f0b921c
Compare
|
/bot run |
|
PR_Github #53554 [ run ] triggered by Bot. Commit: |
|
PR_Github #53554 [ run ] completed with state
|
|
/bot run |
1 similar comment
|
/bot run |
|
PR_Github #53579 [ run ] triggered by Bot. Commit: |
|
PR_Github #53580 [ run ] triggered by Bot. Commit: |
|
PR_Github #53579 [ run ] completed with state |
|
PR_Github #53580 [ run ] completed with state
|
…ditionalGeneration The Kimi-K2.5 multimodal wrapper KimiK25ForConditionalGeneration holds the text backbone as self.llm but does not delegate vocab_size_padded to it. When guided_decoding_backend is set (e.g. xgrammar), py_executor_creator.py reads model_engine.model.vocab_size_padded and raises AttributeError. Add a delegating property matching the pattern used by every other VLM wrapper (qwen2vl, qwen3vl, gemma3vl, llava_next, mistral, phi4mm, etc.). Signed-off-by: tensorrt-cicd <90828364+tensorrt-cicd@users.noreply.github.com>
f0b921c to
8129b65
Compare
|
/bot run |
|
PR_Github #53748 [ run ] triggered by Bot. Commit: |
|
PR_Github #53748 [ run ] completed with state
|
|
/bot run |
|
PR_Github #53801 [ run ] triggered by Bot. Commit: |
|
PR_Github #53801 [ run ] completed with state
|
|
/bot run |
|
PR_Github #53862 [ run ] triggered by Bot. Commit: |
|
PR_Github #53862 [ run ] completed with state |
The rc18->rc19 rebase kept our rc18-based modeling_kimi_k25.py wholesale, which missed rc19's own evolution of the (shared-lineage) file: - NVIDIA#13864 (EPD): get_prompt_token_ids() -> build_disagg_prefill_multimodal_inputs() returning DisaggPrefillMultimodalInputs. rc19's inputs/registry.py now REQUIRES this method for multimodal disagg; our old file only had the removed get_prompt_token_ids -> disaggregated multimodal Kimi would fail registry validation. - NVIDIA#14741: removed DI's lazy slow-tokenizer shim as redundant. - NVIDIA#15219: native vocab_size_padded delegating property. Re-base our file on rc19's modeling_kimi_k25.py and re-apply only the DI deltas that rc19 lacks: - tokenizer denylist discard ("kimi_k25") for correct gapped special-token ids (kept even though the slow-tokenizer shim was dropped; harmless no-op if upstream resolves the denylist entry); - root-logger filter silencing the per-encode "Calling super().encode" spam; - vision-tower MLP row chunking (TLLM_KIMI_VISION_MLP_CHUNK) to bound peak activation on large images; - keep MoonViT3d LayerNorm params in float32 (TRT-LLM LayerNorm.forward upcasts activations -> bf16 weights crash F.layer_norm); - data.image placeholder deficit top-up (drop the now-removed _ensure_k25_slow_tokenizer call); - spec-decode delegations rc19 lacks (spec_worker, spec_config, set_guided_decoder; rc19 already forwards the rest); - preserve original_input_ids across fuse_input_embeds for the Eagle3 drafter. Net +110 lines over rc19 base. Aggregated and disaggregated multimodal paths both supported now. Needs a vision smoke before prod. Signed-off-by: Bo <bo@deepinfra.com>
Summary
KimiK25ForConditionalGeneration(multimodal wrapper aroundself.llm = DeepseekV3ForCausalLM) does not expose avocab_size_paddedproperty, unlike every other VLM wrapper.@property def vocab_size_padded(self) -> int: return self.llm.vocab_size_paddedtoKimiK25ForConditionalGeneration, matching the pattern inmodeling_qwen2vl.py,modeling_qwen3vl.py, etc.Test plan
Links
Summary by CodeRabbit