examples/llm-api/quickstart_multimodal.py (1)

221-228: Avoid dumping full logits to stdout; print shapes/summaries to prevent massive logs and slowdown

Raw context/generation logits can be extremely large (seq_len × vocab_size × beams), which can flood stdout, slow runs, and OOM terminals. Summarize instead and be resilient to missing attributes.

-        if args.return_context_logits:
-            print(f"[{i}] Context logits: {output.context_logits}")
-        if args.return_generation_logits:
-            print(
-                f"[{i}] Generation logits: {output.outputs[0].generation_logits}"
-            )
-        if args.logprobs:
-            print(f"[{i}] Logprobs: {output.outputs[0].logprobs}")
+        if getattr(args, "return_context_logits", False):
+            ctx = getattr(output, "context_logits", None)
+            if ctx is None:
+                print(f"[{i}] Context logits: None")
+            else:
+                shape = getattr(ctx, "shape", None)
+                dtype = getattr(ctx, "dtype", type(ctx))
+                print(f"[{i}] Context logits: shape={tuple(shape) if shape is not None else 'unknown'}, dtype={dtype}")
+
+        if getattr(args, "return_generation_logits", False):
+            gen = getattr(output.outputs[0], "generation_logits", None)
+            if gen is None:
+                print(f"[{i}] Generation logits: None")
+            else:
+                shape = getattr(gen, "shape", None)
+                dtype = getattr(gen, "dtype", type(gen))
+                print(f"[{i}] Generation logits: shape={tuple(shape) if shape is not None else 'unknown'}, dtype={dtype}")
+
+        if getattr(args, "logprobs", False):
+            lps = getattr(output.outputs[0], "logprobs", None)
+            if isinstance(lps, list):
+                head = ", ".join(f"{x:.4f}" for x in lps[:5])
+                more = "..." if len(lps) > 5 else ""
+                print(f"[{i}] Logprobs: n={len(lps)}, head=[{head}{more}]")
+            else:
+                print(f"[{i}] Logprobs: {lps}")

tensorrt_llm/_torch/attention_backend/interface.py (1)

506-508: mRoPE section: add light validation and docstring clarity

Good addition. To make failures obvious when a wrong shape is passed from model wiring, add validation in __post_init__ for mrope_section when present (e.g., positive ints, non-empty, optionally sum <= rope.dim) and briefly document expected semantics (Qwen2/2.5-VL commonly uses three sections). Keep it optional to avoid over-constraining new models.

Example guard (illustrative; adapt location accordingly):

if self.type.is_rope() and self.mrope_section is not None:
    assert all(isinstance(x, int) and x > 0 for x in self.mrope_section), "mrope_section must be positive ints"
    if self.rope and isinstance(self.rope.dim, int):
        assert sum(self.mrope_section) <= self.rope.dim, "sum(mrope_section) must not exceed rope.dim"

tensorrt_llm/_torch/models/modeling_llama.py (2)

182-183: Forward call now passes fewer positional args; prefer keywords to avoid drift

You removed one positional None in the forward_impl(...) call. To make this resilient to future signature changes and to ensure correctness now, pass these trailing optionals by name (e.g., kv_cache_params=None, lora_params=None, skip_attn_scaling=..., attention_sinks=None). This avoids accidental misalignment if forward_impl’s parameter order differs across backends.

Example pattern:

attn_output = self.forward_impl(
    q, k, v,
    attn_metadata,
    attention_mask=attention_mask,
    all_reduce_params=all_reduce_params,
    skip_attn_scaling=skip_attn_scaling,
    attention_sinks=None,
)

Please confirm the exact parameter names.

---

433-435: Min-latency NOPE path: keep call-site robust with keywords

Same concern as above: the parent _forward_nope changed and you removed the mrope_config positional. Pass the trailing parameters by name to guard against parameter drift and improve readability.

return super()._forward_nope(
    position_ids,
    hidden_states,
    attn_metadata,
    attention_mask=attention_mask,
    all_reduce_params=all_reduce_params,
    skip_attn_scaling=skip_attn_scaling,
)

tensorrt_llm/_torch/models/modeling_llama_min_latency.py (1)

433-435: Call into base _forward_nope: use keyword args for clarity

The removed mrope_config positional looks correct. To prevent subtle regressions if _forward_nope evolves, pass the last arguments by name.

-        return super()._forward_nope(position_ids, hidden_states, attn_metadata,
-                                     attention_mask, all_reduce_params,
-                                     skip_attn_scaling)
+        return super()._forward_nope(
+            position_ids,
+            hidden_states,
+            attn_metadata,
+            attention_mask=attention_mask,
+            all_reduce_params=all_reduce_params,
+            skip_attn_scaling=skip_attn_scaling,
+        )

tensorrt_llm/inputs/multimodal.py (2)

388-403: Path-filtered device move: validate dict type; behavior on empty list

Good defensive check that multimodal_data is a dict when keyword is provided. Consider treating keyword=[] as a no-op explicitly (currently it is), and optionally log when a path is not found to ease debugging (silent skip is fine for “Chill”).

---

405-448: Helper mutates input dict in place; document this or deep-copy

result = data and subsequent assignments update the original dict. This is efficient, but note it in the docstring to avoid surprises for callers reusing the dict elsewhere. Alternatively, shallow-copy the parent dict(s) before mutation if you want isolation, at a small cost.

Example shallow-copy at top:

-        result = data
+        result = dict(data)

And copy nested parents before mutation if needed.

tests/unittest/_torch/multimodal/test_share_multiparams.py (1)

149-167: Selective to_device path filter works; consider minor robustness tweaks

The new test validates the dot-path filtering behavior and confirms non-targeted tensors remain on CPU. Looks good. Two optional hardening ideas:

• Add a skip guard for environments without CUDA to avoid false negatives on CPU-only CI.
• Consider adding one more assertion for a non-existent path (e.g., "image.not_there") to confirm it’s safely ignored.

Example (optional):

@unittest.skipIf(not torch.cuda.is_available(), "CUDA required for this test")
def test_to_device_with_keyword(self):
    ...

tensorrt_llm/_torch/models/checkpoints/__init__.py (1)

18-19: Keep all tidy (optional)

Appending the new symbol is fine. Consider keeping all sorted alphabetically to reduce churn in future diffs.

tensorrt_llm/_torch/modules/attention.py (1)

267-279: Initialize MRotaryEmbedding defensively when mrope_section may be absent

The MRope path looks correct. To avoid AttributeError with older configs that don’t carry pos_embd_params.mrope_section, construct MRotaryEmbedding without the parameter when it’s missing (class has a sensible default). This preserves backward compatibility.

Apply this diff within the block:

-            if self.pos_embd_params.type.is_mrope():
-                self.rotary_emb = MRotaryEmbedding(
-                    self.pos_embd_params.rope,
-                    head_dim=self.head_dim,
-                    is_neox=self.pos_embd_params.is_neox,
-                    mrope_section=self.pos_embd_params.mrope_section,
-                )
+            if self.pos_embd_params.type.is_mrope():
+                if hasattr(self.pos_embd_params, "mrope_section"):
+                    self.rotary_emb = MRotaryEmbedding(
+                        self.pos_embd_params.rope,
+                        head_dim=self.head_dim,
+                        is_neox=self.pos_embd_params.is_neox,
+                        mrope_section=self.pos_embd_params.mrope_section,
+                    )
+                else:
+                    self.rotary_emb = MRotaryEmbedding(
+                        self.pos_embd_params.rope,
+                        head_dim=self.head_dim,
+                        is_neox=self.pos_embd_params.is_neox,
+                    )

tensorrt_llm/_torch/pyexecutor/resource_manager.py (2)

484-484: Fix typo and wrap long NOTE to satisfy Ruff E501

There’s a minor typo (“mrop_config”) and the line exceeds 120 chars. Reflow into two lines and fix the spelling.

Apply this diff:

-            # NOTE: Planning to get dummy_data from each model. Before that, we need to add dummy mrop_config to the request here.
+            # NOTE: Planning to get dummy_data from each model.
+            # Before that, add a dummy mrope_config to the request here.

---

493-498: Dummy mrope_position_deltas shape looks fine; consider setdefault if py_multimodal_data may exist later

For dummy requests it’s safe to overwrite py_multimodal_data. If this function ever reuses existing requests, prefer setdefault to avoid clobbering other dummy modalities.

Alternative (for future-proofing):

req.py_multimodal_data = getattr(req, "py_multimodal_data", {}) or {}
req.py_multimodal_data.setdefault("mrope_config", {})["mrope_position_ids"] = dummy_mrope_position_ids
...

tensorrt_llm/_torch/pyexecutor/cuda_graph_runner.py (1)

61-69: Consider using a factory function for MultimodalParams creation.

The multimodal_params initialization with default mrope_config could be extracted into a helper method for better readability and reusability.

Consider refactoring to improve readability:

+    def _create_mrope_multimodal_params(self, batch_size: int, device: str):
+        """Create MultimodalParams with default mrope configuration."""
+        return [
+            MultimodalParams(
+                multimodal_data={
+                    "mrope_config": {
+                        "mrope_position_deltas":
+                        torch.zeros((1, 1), device=device, dtype=torch.int32)
+                    }
+                }) for _ in range(batch_size)
+        ]
+
     def __init__(self, ...):
         ...
-        self.multimodal_params = [
-            MultimodalParams(
-                multimodal_data={
-                    "mrope_config": {
-                        "mrope_position_deltas":
-                        torch.zeros((1, 1), device=device, dtype=torch.int32)
-                    }
-                }) for _ in range(batch_size)
-        ] if self.use_mrope else []
+        self.multimodal_params = self._create_mrope_multimodal_params(
+            batch_size, device) if self.use_mrope else []

tensorrt_llm/_torch/modules/rotary_embedding.py (1)

167-170: Consider adding a warning for unexpected 2D position_ids in mRoPE mode.

Since MRotaryEmbedding is specifically for mRoPE, receiving 2D position_ids might indicate a configuration issue.

Consider adding a warning when falling back to standard RoPE:

         else:
             # Fallback to the original RoPE where position_ids is 2D for dummy requests
+            import warnings
+            warnings.warn(
+                "MRotaryEmbedding received 2D position_ids, falling back to standard RoPE. "
+                "This may indicate a configuration issue.",
+                RuntimeWarning,
+                stacklevel=2
+            )
             cos_sin = self.rotary_cos_sin[position_ids.view(-1)]
             cos, sin = cos_sin[:, 0, :], cos_sin[:, 1, :]

tensorrt_llm/_torch/pyexecutor/model_engine.py (2)

1287-1306: Fix line length issues for better readability.

The code logic is correct, but several lines exceed the 120-character limit, affecting readability.

Apply this formatting fix:

             if self.use_mrope:
                 ctx_mrope_position_ids = multimodal_params.multimodal_data[
-                    'mrope_config'][
-                        'mrope_position_ids'][:, :,
-                                              begin_compute:begin_compute +
-                                              len(prompt_tokens)]
+                    'mrope_config']['mrope_position_ids'][
+                        :, :, begin_compute:begin_compute + len(prompt_tokens)]
                 mrope_position_ids.append(ctx_mrope_position_ids)

                 # TODO: Visit later to decide the appropriate position of sending multimodal data & selectively sending multimodal data
                 multimodal_params.to_device(
                     "multimodal_data",
                     "cuda",
                     pin_memory=True,
-                    keyword=getattr(self.model, "multimodal_data_device_paths",
-                                    None))
+                    keyword=getattr(
+                        self.model, "multimodal_data_device_paths", None))

---

1441-1465: Consider extracting mRoPE position ID generation logic.

The mRoPE position ID generation and multimodal parameter handling could be extracted into a helper method for better maintainability.

Consider refactoring into a helper method:

+    def _prepare_generation_mrope_params(
+            self, past_seen_token_num: int,
+            multimodal_data: dict) -> Tuple[torch.Tensor, MultimodalParams]:
+        """Prepare mRoPE parameters for generation requests."""
+        mrope_position_deltas = multimodal_data[
+            'mrope_config']['mrope_position_deltas']
+        gen_mrope_position_ids = (
+            past_seen_token_num + mrope_position_deltas
+        ).expand(3, 1, 1)
+        
+        multimodal_params = MultimodalParams(multimodal_data=multimodal_data)
+        multimodal_params.strip_for_generation()
+        if multimodal_params.has_content():
+            multimodal_params.to_device(
+                "multimodal_data", "cuda", pin_memory=True,
+                keyword=["mrope_config.mrope_position_deltas"])
+        return gen_mrope_position_ids, multimodal_params
+
                 # Multimodal
-                multimodal_params = MultimodalParams(
-                    multimodal_data=request.py_multimodal_data)
-                multimodal_params.strip_for_generation()
-                if multimodal_params.has_content():
+                if request.py_multimodal_data and self.use_mrope:
+                    gen_mrope_position_ids, multimodal_params = (
+                        self._prepare_generation_mrope_params(
+                            past_seen_token_num, request.py_multimodal_data))
+                    mrope_position_ids.append(gen_mrope_position_ids)
+                    multimodal_params_list.append(multimodal_params)

tests/unittest/_torch/modeling/test_modeling_qwen2_5vl.py (2)

101-102: Fix line length in repr method.

The return statement exceeds the 120-character limit.

Split the string for better readability:

     def __repr__(self) -> str:
-        return f"modality:{self.modality.lower()}-use_cuda_graph:{self.use_cuda_graph}-disable_fuse_rope:{self.disable_fuse_rope}"
+        return (f"modality:{self.modality.lower()}-"
+                f"use_cuda_graph:{self.use_cuda_graph}-"
+                f"disable_fuse_rope:{self.disable_fuse_rope}")

---

134-136: Consider adding error handling for tokenizer initialization.

The tokenizer loading could fail if the model path is invalid.

Add error handling:

     def get_inputprocessor(self, model_path):
-        tokenizer = AutoTokenizer.from_pretrained(model_path)
-        return create_input_processor(model_path, tokenizer=tokenizer)
+        try:
+            tokenizer = AutoTokenizer.from_pretrained(model_path)
+            return create_input_processor(model_path, tokenizer=tokenizer)
+        except Exception as e:
+            raise RuntimeError(f"Failed to load tokenizer from {model_path}: {e}")

tensorrt_llm/_torch/models/modeling_qwen2vl.py (8)

62-89: Align docstring with return shape and ensure consistent dtype/shape.

• Docstring says mrope_position_deltas has shape (batch_size), but the code returns (B, 1). Keep them consistent.
• Consider using torch.int32 for deltas at the source to avoid later casting churn.

Apply:

-        Returns:
-            position_ids: A tensor of shape (3, batch_size, sequence_length)
-            mrope_position_deltas: A tensor of shape (batch_size)
+        Returns:
+            position_ids: torch.IntTensor of shape (3, batch_size, sequence_length)
+            mrope_position_deltas: torch.IntTensor of shape (batch_size, 1)

And when returning in the no-vision path:

-                mrope_position_deltas = torch.zeros(
+                mrope_position_deltas = torch.zeros(
                     [input_ids.shape[0], 1],
                     device=input_ids.device,
-                    dtype=input_ids.dtype,
+                    dtype=torch.int32,
                 )

Also applies to: 232-235, 296-313

---

135-146: Avoid shadowing input_ids parameter inside loop.

Reusing input_ids for the per-row tensor shadows the function parameter and risks confusion.

Apply:

-        for i, input_ids in enumerate(total_input_ids):
-            input_ids = input_ids[attention_mask[i] == 1]
+        for i, seq_input_ids in enumerate(total_input_ids):
+            seq_input_ids = seq_input_ids[attention_mask[i] == 1]
             image_nums, video_nums = 0, 0
             vision_start_indices = torch.argwhere(
-                input_ids == vision_start_token_id).squeeze(1)
-            vision_tokens = input_ids[vision_start_indices + 1]
+                seq_input_ids == vision_start_token_id).squeeze(1)
+            vision_tokens = seq_input_ids[vision_start_indices + 1]
-            input_tokens = input_ids.tolist()
+            input_tokens = seq_input_ids.tolist()

---

303-313: Make CPU clones explicit and unify dtypes.

• Keeping mrope_position_ids as int64 is fine for indexing later, but spell out intent.
• mrope_position_deltas is cast to int32; consider casting IDs to int64 explicitly to avoid implicit conversions.

Apply:

-        mrope_config['mrope_position_ids'] = mrope_position_ids.to(
-            'cpu').clone()
+        mrope_config['mrope_position_ids'] = mrope_position_ids.to('cpu', dtype=torch.long).clone()

---

349-358: Remove commented debug prints and long lines (Ruff E501).

Dead debug prints add noise and break line-length checks. Please delete them.

Apply:

-        # print(f"fused_input_ids: {fused_input_ids}")
-        # print(f"multimodal_data['image']['pixel_values'].shape: {multimodal_data['image']['pixel_values'].shape}")
-        # print(f"multimodal_data['image']['image_grid_thw']: {multimodal_data['image']['image_grid_thw']}")
-        # print(f"multimodal_data['mrope_config']['mrope_position_ids'].shape: {multimodal_data['mrope_config']['mrope_position_ids'].shape}")
-        # print(f"multimodal_data['mrope_config']['mrope_position_deltas'].shape: {multimodal_data['mrope_config']['mrope_position_deltas'].shape}")

---

521-533: Replace print statements with logger and avoid dumping giant key lists.

Use logger.debug and optionally gate behind a verbosity flag; dumping entire weights.keys() is noisy.

Apply:

-        if not DISAGG:
-            print(f"weights.keys(): {weights.keys()}")
-            vision_encoder_weights = filter_weights("visual", weights)
-            print(
-                f"vision_encoder_weights.keys(): {vision_encoder_weights.keys()}"
-            )
+        if not DISAGG:
+            vision_encoder_weights = filter_vision_weights("visual", weights)
+            logger.debug("Loaded %d vision weights", len(vision_encoder_weights))

---

32-34: Global naming: prefer G_ prefix for globals.

Per guidelines, globals should be UPPER_SNAKE_CASE prefixed with G_. Consider renaming DISAGG to G_DISAGG for consistency.

Sample:

-DISAGG = os.getenv('TLLM_MULTIMODAL_DISAGGREGATED', '0') == '1'
+G_DISAGG = os.getenv('TLLM_MULTIMODAL_DISAGGREGATED', '0') == '1'

Update references accordingly.

---

356-357: Resolve Ruff E501: wrap or reflow long lines.

A few lines exceed 120 characters. Please wrap them to satisfy linters.

You can break the long comment at Line 477 and remove the debug prints at Lines 356–357 as suggested earlier. For other occurrences, wrap strings/comments across lines.

Also applies to: 477-479, 566-569

---

296-313: Add minimal docstrings for public helpers.

get_mrope_config and prepare_mrope_config are externally consumed via MultimodalParams; add short Google-style docstrings for clarity.

Also applies to: 609-711

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

Learnt from: amitz-nv
PR: NVIDIA/TensorRT-LLM#7033
File: tensorrt_llm/_torch/pyexecutor/model_engine.py:0-0
Timestamp: 2025-08-19T12:45:11.997Z
Learning: In tensorrt_llm/_torch/pyexecutor/model_engine.py, DoRA (Delta Orthogonal Rank Adaptation) functionality was removed from the PyTorch flow to eliminate issues with inverted DoRA detection logic. The original is_dora condition was checking if scaling_vec_pointer == 0, which was potentially incorrect.