tensorrt_llm/inputs/utils.py (1)

456-462: Handle potential empty placeholder counts gracefully.

The current implementation assumes all mm_placeholder_count entries have at least one key when creating image placeholders. If a message has no multimodal content, this could lead to unexpected behavior.

Consider adding a check for empty placeholder counts:

 def handle_placeholder_exceptions(model_type: str,
                                   conversation: list[ConversationMessage],
                                   mm_placeholder_counts: list[dict[str, int]]):
     if model_type == "llava_next":
         # we need to convert the flattened content back to conversation format
         for conv, mm_placeholder_count in zip(conversation,
                                               mm_placeholder_counts):
             conv["content"] = [{"type": "text", "text": conv["content"]}, \
-                *[{"type": "image"} for _ in mm_placeholder_count]]
+                *[{"type": "image"} for _ in mm_placeholder_count if mm_placeholder_count]]

tensorrt_llm/evaluate/lm_eval.py (2)

161-161: Revise docstring to accurately reflect TRT-LLM's unlimited image capability

The comment about "currently unlimited in TRT-LLM" contradicts the parameter's purpose and the default value of 999 from lm_eval. Consider clarifying the actual limitation.

-            max_images: Maximum number of images per prompt (currently unlimited in TRT-LLM), set to 999 from lm_eval's default value.
+            max_images: Maximum number of images per prompt (default: 999 from lm_eval)

---

321-344: Simplify nested function and improve readability

The _adjust_config function is defined inside __init__ but could be a class method or module-level function for better organization. Also, the recursive structure could be clearer.

Consider extracting this as a private class method:

-        # Adopted from https://github.com/EleutherAI/lm-evaluation-harness/blob/7f04db12d2f8e7a99a0830d99eb78130e1ba2122/lm_eval/evaluator.py#L290
-        def _adjust_config(task_dict, random_seed):
-            adjusted_task_dict = {}
-            for task_name, task_obj in task_dict.items():
-                if isinstance(task_obj, dict):
-                    adjusted_task_dict = {
-                        **adjusted_task_dict,
-                        **{
-                            task_name: _adjust_config(task_obj, random_seed)
-                        },
-                    }
-                else:
-                    # NOTE: Few-shot random seed
-                    task_obj.set_config(key="num_fewshot", value=0)
-                    task_obj.set_fewshot_seed(seed=random_seed)
-                    adjusted_task_dict[task_name] = task_obj
-
-                    # NOTE: Shuffle dataset
-                    data = adjusted_task_dict[task_name].dataset
-                    for split in data.keys():
-                        data[split] = data[split].shuffle(random_seed)
-
-            return adjusted_task_dict
-
-        self.task_dict = _adjust_config(self.task_dict, random_seed)
+        self.task_dict = self._adjust_config(self.task_dict, random_seed)

And add as a class method:

    def _adjust_config(self, task_dict, random_seed):
        """Adjust task configuration for evaluation.
        
        Adopted from: https://github.com/EleutherAI/lm-evaluation-harness/blob/7f04db12d2f8e7a99a0830d99eb78130e1ba2122/lm_eval/evaluator.py#L290
        """
        adjusted_task_dict = {}
        for task_name, task_obj in task_dict.items():
            if isinstance(task_obj, dict):
                # Recursively adjust nested task dictionaries
                adjusted_task_dict[task_name] = self._adjust_config(task_obj, random_seed)
            else:
                # Configure individual task
                task_obj.set_config(key="num_fewshot", value=0)
                task_obj.set_fewshot_seed(seed=random_seed)
                adjusted_task_dict[task_name] = task_obj
                
                # Shuffle dataset splits
                data = task_obj.dataset
                for split in data.keys():
                    data[split] = data[split].shuffle(random_seed)
                    
        return adjusted_task_dict

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.

tensorrt_llm/evaluate/lm_eval.py (6)

651-658: Consider making default values more flexible

Both is_multimodal and apply_chat_template default to True, which creates redundancy since multimodal mode automatically enables chat templates (line 303). Consider keeping is_multimodal as True for MMMU but making apply_chat_template default to False to avoid confusion.

     @click.option("--apply_chat_template",
                   is_flag=True,
-                  default=True,
+                  default=False,
-                  help="Whether to apply chat template.")
+                  help="Whether to apply chat template (automatically enabled for multimodal).")

---

134-139: Move imports to the top of the file

These imports should be moved to the top of the file to follow Python's import organization conventions (PEP 8). Currently they're placed after class definitions, which violates standard practice.

-from tensorrt_llm.inputs import (ConversationMessage, MultimodalDataTracker,
-                                 add_multimodal_placeholders,
-                                 convert_image_mode)
-from tensorrt_llm.inputs.utils import \
-    apply_chat_template as trtllm_apply_chat_template

Add these imports at the top of the file after line 39:

from tensorrt_llm.inputs import (ConversationMessage, MultimodalDataTracker,
                                 add_multimodal_placeholders,
                                 convert_image_mode)
from tensorrt_llm.inputs.utils import apply_chat_template as trtllm_apply_chat_template

---

141-180: Consider improving model type extraction robustness

The _get_model_type method accesses a private attribute _hf_model_dir and has a catch-all exception handler. Consider checking for both private and public attribute variants, and catch only expected exceptions.

     def _get_model_type(self, llm: Union[LLM, PyTorchLLM]) -> str:
         """Extract model type from the model configuration."""
         try:
-            config_path = os.path.join(llm._hf_model_dir, 'config.json')
+            # Try to get the model directory path
+            if hasattr(llm, '_hf_model_dir'):
+                model_dir = llm._hf_model_dir
+            elif hasattr(llm, 'hf_model_dir'):
+                model_dir = llm.hf_model_dir
+            else:
+                logger.warning("Could not find model directory attribute")
+                return "unknown"
+                
+            config_path = os.path.join(model_dir, 'config.json')
             with open(config_path, 'r') as f:
                 config = json.load(f)
-            return config['model_type']
-        except (KeyError, FileNotFoundError, json.JSONDecodeError) as e:
+            return config.get('model_type', 'unknown')
+        except (FileNotFoundError, json.JSONDecodeError) as e:
             logger.warning(f"Could not determine model type: {e}")
             return "unknown"

---

200-204: Remove incomplete interleave implementation

The interleaved text and image logic is incomplete with just a split operation and ellipsis. This dead code should be removed or properly implemented.

-            if self.interleave:
-                # TODO: Implement interleaved text and image.
-                text.split(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER)
-                ...
-            else:
-                text = text.replace(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER, "")
+            # TODO: Implement interleaved text and image support
+            text = text.replace(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER, "")

---

256-259: Add error handling for missing or invalid image data

The code assumes media_data["visual"] exists and contains valid images. Add validation to handle cases where the visual data might be missing or malformed.

-            images = [
-                convert_image_mode(img, "RGB") for img in media_data["visual"]
-            ]
+            images = []
+            if media_data and "visual" in media_data:
+                try:
+                    images = [
+                        convert_image_mode(img, "RGB") for img in media_data["visual"]
+                    ]
+                except Exception as e:
+                    logger.error(f"Failed to process visual data: {e}")
+                    raise ValueError(f"Invalid visual data in request: {e}")

---

620-621: Pass is_multimodal=True explicitly for MMMU evaluator

The MMMU evaluator should explicitly pass is_multimodal=True to the parent class to ensure multimodal support is enabled.

     def __init__(self, **kwargs):
+        kwargs['is_multimodal'] = True
         super().__init__("mmmu_val", **kwargs)

tensorrt_llm/inputs/utils.py (2)

74-81: Consider using convert_image_mode for consistency

The function currently uses image.convert('RGB') directly, while the rest of the codebase uses the new convert_image_mode helper. Consider using it here for consistency.

     if isinstance(image, Image.Image):
-        return image.convert('RGB')
+        return convert_image_mode(image, 'RGB')

---

98-106: Consider using convert_image_mode for consistency

Similar to the synchronous version, consider using convert_image_mode for consistency.

     if isinstance(image, Image.Image):
-        return image.convert('RGB')
+        return convert_image_mode(image, 'RGB')

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

tensorrt_llm/evaluate/lm_eval.py (5)

36-38: Fix import ordering: Place all imports at the top of the file

These imports violate PEP 8 by appearing after class definitions. Move them to the top of the file with the other imports.

Remove lines 36-38 and add them after line 26:

 import tensorrt_llm.profiler as profiler
+from tensorrt_llm.inputs import (ConversationMessage, MultimodalDataTracker,
+                                 add_multimodal_placeholders, convert_image_mode,
+                                 prompt_inputs)
+from tensorrt_llm.inputs.utils import apply_chat_template as trtllm_apply_chat_template
-from tensorrt_llm.inputs import prompt_inputs

Also remove the duplicate import on line 26.

---

167-176: Add robust error handling for model type extraction

The method accesses a private attribute _hf_model_dir which could break with internal changes. Additionally, the generic exception handling might mask important errors.

 def _get_model_type(self, llm: Union[LLM, PyTorchLLM]) -> str:
     """Extract model type from the model configuration."""
     try:
-        config_path = os.path.join(llm._hf_model_dir, 'config.json')
+        # Try to get the model directory path
+        if hasattr(llm, 'hf_model_dir'):
+            model_dir = llm.hf_model_dir
+        elif hasattr(llm, '_hf_model_dir'):
+            model_dir = llm._hf_model_dir
+        else:
+            logger.warning("Could not find model directory attribute")
+            return "unknown"
+            
+        config_path = os.path.join(model_dir, 'config.json')
         with open(config_path, 'r') as f:
             config = json.load(f)
-        return config['model_type']
-    except (KeyError, FileNotFoundError, json.JSONDecodeError) as e:
+        return config.get('model_type', 'unknown')
+    except (FileNotFoundError, json.JSONDecodeError) as e:
         logger.warning(f"Could not determine model type: {e}")
         return "unknown"

---

196-201: Remove incomplete interleave implementation

The interleaved text and image logic contains dead code with just a split operation and ellipsis. This should be removed or properly implemented.

-            if self.interleave:
-                # TODO: Implement interleaved text and image.
-                text.split(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER)
-                ...
-            else:
-                text = text.replace(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER, "")
+            # TODO: Implement interleaved text and image support
+            text = text.replace(LM_EVAL_DEFAULT_IMAGE_PLACEHOLDER, "")

---

253-255: Add validation for visual data in multimodal requests

The code assumes media_data["visual"] exists and contains valid images without any validation.

-            images = [
-                convert_image_mode(img, "RGB") for img in media_data["visual"]
-            ]
+            images = []
+            if media_data and "visual" in media_data:
+                try:
+                    images = [
+                        convert_image_mode(img, "RGB") for img in media_data["visual"]
+                    ]
+                except Exception as e:
+                    logger.error(f"Failed to process visual data: {e}")
+                    raise ValueError(f"Invalid visual data in request: {e}")
+            else:
+                logger.warning("No visual data found in multimodal request")

---

606-607: MMMU should explicitly set multimodal flag

The MMMU evaluator should explicitly pass is_multimodal=True to ensure multimodal support is always enabled.

 def __init__(self, **kwargs):
+    kwargs['is_multimodal'] = True
     super().__init__("mmmu_val", **kwargs)

tensorrt_llm/evaluate/lm_eval.py (1)

149-158: Fix line length issue in docstring

Line 157 exceeds the 120 character limit.

-            max_images: Maximum number of images per prompt (currently unlimited in TRT-LLM), set to 999 from lm_eval's default value.
+            max_images: Maximum number of images per prompt (currently unlimited in TRT-LLM),
+                set to 999 from lm_eval's default value.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: moraxu
PR: NVIDIA/TensorRT-LLM#6303
File: tests/integration/test_lists/qa/examples_test_list.txt:494-494
Timestamp: 2025-07-28T17:06:08.621Z
Learning: In TensorRT-LLM testing, it's common to have both CLI flow tests (test_cli_flow.py) and PyTorch API tests (test_llm_api_pytorch.py) for the same model. These serve different purposes: CLI flow tests validate the traditional command-line workflow, while PyTorch API tests validate the newer LLM API backend. Both are legitimate and should coexist.