[TRTLLM-11759][fix] Reduce peak host memory during NemotronH_Nano_VL_V2 init - #13283
Conversation
Signed-off-by: Pamela <179191831+pamelap-nvidia@users.noreply.github.com>
d5577f3 to
60c9fe5
Compare
|
/bot run |
|
PR_Github #44768 [ run ] triggered by Bot. Commit: |
|
PR_Github #44768 [ run ] completed with state
|
|
/bot run --disable-fail-fast |
|
PR_Github #44792 [ run ] triggered by Bot. Commit: |
📝 WalkthroughWalkthroughRefactored NemotronH_Nano_VL_V2 to implement lazy initialization of vision and sound encoders within load_weights instead of init, adding CUDA placement for these encoders, conditional vision weight loading, and memory management via weights.mark_consumed() calls for resource cleanup. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tensorrt_llm/_torch/models/modeling_nemotron_nano.py (1)
1792-1793: Use consistent type annotation style.Line 1792 uses
Optional[]while line 1793 uses the|syntax. For consistency with Python 3.10+ style, preferNanoV2VLVisionEncoder | Noneon line 1792.♻️ Suggested fix for consistency
- self.vision_encoder: Optional[NanoV2VLVisionEncoder] = None + self.vision_encoder: NanoV2VLVisionEncoder | None = NoneAs per coding guidelines: "Use the
|syntax instead oftyping.Unionin Python type annotations" (which also applies toOptional[T]being equivalent toT | None).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tensorrt_llm/_torch/models/modeling_nemotron_nano.py` around lines 1792 - 1793, Replace the inconsistent typing for the vision encoder: change the attribute annotation self.vision_encoder: Optional[NanoV2VLVisionEncoder] to use the union pipe syntax (self.vision_encoder: NanoV2VLVisionEncoder | None) so it matches the style used by self.sound_encoder (ProjectedParakeet | None) and other Python 3.10+ annotations in the class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tensorrt_llm/_torch/models/modeling_nemotron_nano.py`:
- Around line 1792-1793: Replace the inconsistent typing for the vision encoder:
change the attribute annotation self.vision_encoder:
Optional[NanoV2VLVisionEncoder] to use the union pipe syntax
(self.vision_encoder: NanoV2VLVisionEncoder | None) so it matches the style used
by self.sound_encoder (ProjectedParakeet | None) and other Python 3.10+
annotations in the class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0fccb5f5-e6ab-4e93-83a5-7bb79214eee4
📒 Files selected for processing (1)
tensorrt_llm/_torch/models/modeling_nemotron_nano.py
|
/bot skip --comment "No available machines for DGX_B200 tests. All other tests passed" |
|
PR_Github #44821 [ skip ] triggered by Bot. Commit: |
|
PR_Github #44821 [ skip ] completed with state |
Iterates on the trtllm-model-onboard-multimodal SKILL.md per reviewer feedback on PR NVIDIA#13842: - Tighten frontmatter description to a trigger-only summary; move implementation detail into the body. - "vision tower" -> "multimodal encoder(s)" throughout where the term is modality-agnostic (audio / video as well). - Trim system-map diagrams: drop file paths and private-method names that belong to debugging/perf rather than onboarding; restore the contributor-facing public APIs (find_input_mm_embeds, prepare_mrope_config, get_multimodal_embeddings cache hits, mrope CPU staging) and add explicit Key invariants. - Clarify "blocking" vs to_thread wording so it no longer contradicts the diagram's async/threaded staging. - Define what a shared-tensor "handle" actually is (small dict + IPC/shm key) instead of the jargon "handle-shaped". - Mark the get_text_with_mm_placeholders / expand_prompt_token_ids_for_mm fast path as optional pending the planned refactor. - Add Phase 4 host-memory guidance from PR NVIDIA#13283 (lazy encoder construction outside MetaInitMode + weights.mark_consumed for incremental mmap shard release). - Drop the duplicated compose-from rule under the dedicated Reuse section; expand the encoder-length / placeholder-count invariant with the chunked-prefill cache failure mode. - Expand "Be parsimonious" guidance for TestModelingMultimodal scenarios so CI runtime is bounded. - Add quickstart_multimodal.py as the first line of defense before trtllm-serve, plus the chunked-prefill log grep verification step. - Clarify AutoModelForCausalLM is TRT-LLM's, not transformers'. - Compute modules table: drop entries that are obvious or stale-prone, keep only non-obvious wiring (Linear TP/quant kwargs, Attention dual use for LLM/encoder, MLP/GatedMLP fused gate/up, mRoPE vs RotaryEmbedding split, MoE pointer to MOE_DEVELOPER_GUIDE.md). - "Port to TRT-LLM modules" is required (not preferred); spell out the two reasons (perf + transformers version coupling) and ban HF module/computation imports outright with one Qwen2-VL exception. - Add Contract 4: batch the multimodal encoder across requests in one forward (concat pixel_values + per-image seq_lens; nsys audit shows one wide block, not N narrow ones). - Replace Phase 2 model-wrapper template with the LlavaNext / Gemma3VL pattern (single class, super().__init__, text_config-based AutoModel.from_config, post_config, vocab_size_padded / infer_max_seq_len). Note Base/non-Base split (Qwen2VL/Qwen3VL) as a multi-variant implementation detail, not the default. Net change: +212/-160. File grows from 386 to 438 lines as new content (Contract 4, host-memory guidance, default Phase 2 template, quickstart verification, two-reason port argument) outweighs trims. Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
Iterates on the trtllm-model-onboard-multimodal SKILL.md per reviewer feedback on PR NVIDIA#13842: - Tighten frontmatter description to a trigger-only summary; move implementation detail into the body. - "vision tower" -> "multimodal encoder(s)" throughout where the term is modality-agnostic (audio / video as well). - Trim system-map diagrams: drop file paths and private-method names that belong to debugging/perf rather than onboarding; restore the contributor-facing public APIs (find_input_mm_embeds, prepare_mrope_config, get_multimodal_embeddings cache hits, mrope CPU staging) and add explicit Key invariants. - Clarify "blocking" vs to_thread wording so it no longer contradicts the diagram's async/threaded staging. - Define what a shared-tensor "handle" actually is (small dict + IPC/shm key) instead of the jargon "handle-shaped". - Mark the get_text_with_mm_placeholders / expand_prompt_token_ids_for_mm fast path as optional pending the planned refactor. - Add Phase 4 host-memory guidance from PR NVIDIA#13283 (lazy encoder construction outside MetaInitMode + weights.mark_consumed for incremental mmap shard release). - Drop the duplicated compose-from rule under the dedicated Reuse section; expand the encoder-length / placeholder-count invariant with the chunked-prefill cache failure mode. - Expand "Be parsimonious" guidance for TestModelingMultimodal scenarios so CI runtime is bounded. - Add quickstart_multimodal.py as the first line of defense before trtllm-serve, plus the chunked-prefill log grep verification step. - Clarify AutoModelForCausalLM is TRT-LLM's, not transformers'. - Compute modules table: drop entries that are obvious or stale-prone, keep only non-obvious wiring (Linear TP/quant kwargs, Attention dual use for LLM/encoder, MLP/GatedMLP fused gate/up, mRoPE vs RotaryEmbedding split, MoE pointer to MOE_DEVELOPER_GUIDE.md). - "Port to TRT-LLM modules" is required (not preferred); spell out the two reasons (perf + transformers version coupling) and ban HF module/computation imports outright with one Qwen2-VL exception. - Add Contract 4: batch the multimodal encoder across requests in one forward (concat pixel_values + per-image seq_lens; nsys audit shows one wide block, not N narrow ones). - Replace Phase 2 model-wrapper template with the LlavaNext / Gemma3VL pattern (single class, super().__init__, text_config-based AutoModel.from_config, post_config, vocab_size_padded / infer_max_seq_len). Note Base/non-Base split (Qwen2VL/Qwen3VL) as a multi-variant implementation detail, not the default. Net change: +212/-160. File grows from 386 to 438 lines as new content (Contract 4, host-memory guidance, default Phase 2 template, quickstart verification, two-reason port argument) outweighs trims. Signed-off-by: yechank <161688079+yechank-nvidia@users.noreply.github.com>
The multimodal NemotronH_Nano_VL_V2 model was hitting MetaInitException during MetaInitMode-guarded init because its HuggingFace-based submodules (nn.RMSNorm / nn.LayerNorm in the RADIO vision encoder, HFParakeetEncoder for the audio tower) use deterministic init ops (ones_, zeros_, fill_, .to(dtype=...), .detach()) that raise MetaInitException on meta tensors. The resulting fallback to regular init:
Fix:
Vision+sound weights are ~2GB combined versus ~33-62GB for the LLM, so allocating them on CPU first and moving to CUDA in load_weights is cheap. MetaInitMode itself is left unchanged, avoiding any risk of silent meta-tensor leaks into runtime buffers.
After the fix, model.to("cuda") becomes a no-op (0.01s vs 225s) since LLM tensors are allocated directly on CUDA from meta placeholders, peak proc_rss drops ~30GB, and "memory used outside torch" drops from ~77GiB to ~54GiB.
It also reduced model init time by avoiding the model init fallback.
Summary by CodeRabbit
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.