Conversation
There was a problem hiding this comment.
Pull request overview
Refactors InternS1 Pro vision model preprocessing by inheriting from the existing Qwen3VL implementation, reducing duplicated preprocessing logic while keeping time-series support.
Changes:
- Enable
trust_remote_code=Truewhen loading the Qwen3 processor. - Simplify/centralize InternS1 Pro preprocessing by subclassing
Qwen3VLModeland only adding time-series-specific handling. - Adjust the FPS fallback warning message used during video preprocessing.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lmdeploy/vl/model/qwen3.py |
Loads the HF processor with trust_remote_code=True and tweaks the FPS fallback warning string. |
lmdeploy/vl/model/interns1_pro.py |
Replaces duplicated Qwen3-like preprocessing with inheritance from Qwen3VLModel, retaining time-series preprocessing/packing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def build_preprocessor(self): | ||
| check_transformers() | ||
| self.processor = AutoProcessor.from_pretrained(self.model_path) | ||
| self.processor = AutoProcessor.from_pretrained(self.model_path, trust_remote_code=True) |
There was a problem hiding this comment.
Enabling trust_remote_code=True allows execution of arbitrary code from the model repository during processor loading. If this is required for Qwen3 processors, consider making it configurable (e.g., via a model/engine option) and/or documenting the security implication so users running untrusted model paths can disable it.
| self.processor = AutoProcessor.from_pretrained(self.model_path, trust_remote_code=True) | |
| trust_remote_code = getattr(self, 'trust_remote_code', False) | |
| if trust_remote_code: | |
| logger.warning( | |
| 'Enabling trust_remote_code=True when loading the Qwen3VL processor. ' | |
| 'This allows execution of arbitrary code from the model repository. ' | |
| 'Disable this option when using untrusted model paths.' | |
| ) | |
| self.processor = AutoProcessor.from_pretrained( | |
| self.model_path, | |
| trust_remote_code=trust_remote_code, | |
| ) |
There was a problem hiding this comment.
We will introduce --trust-remote-code next version
| metadata = params['video_metadata'] | ||
| if metadata.get('fps') is None or metadata['fps'] <= 0: | ||
| logger.warning('Qwen3VL: fps not found or invalid, fallback to 24.') | ||
| logger.warning('fps not found or invalid, fallback to 24.') |
There was a problem hiding this comment.
The warning message lost the model identifier; when multiple VLMs are used in the same process, this becomes hard to attribute in logs. Consider including the model name dynamically (e.g., via type(self).__name__) or a consistent prefix.
Preprocessing code has grown significantly; using inheritance will simplify the interns1 pro file.