Feature/video support in random mm dataset #25963
Conversation
Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
…hen generating random inputs (This is to avoid inserting mm-related tokens which may confuse VLM models) Signed-off-by: Eugene Khvedchenia <[email protected]>
Signed-off-by: Eugene Khvedchenia <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
| # Build the inner sequence by sampling | ||
| # sequentially from the allowed tokens | ||
| inner_seq = allowed_tokens[ | ||
| (offset + index + np.arange(prefix_len)) % len(allowed_tokens) | ||
| ].tolist() |
There was a problem hiding this comment.
In RandomDataset.generate_token_sequence the inner sequence is built with np.arange(prefix_len) and indexed into allowed_tokens. For the common case where prefix_len is 0, this creates an empty token_sequence, causing gen_prompt_decode_to_target_len to backfill almost the entire prompt with random tokens from the full vocabulary, reintroducing special tokens and ignoring the supplied input_len and offsets. This regresses determinism and the intent of filtering out special IDs for every request with input_len > 0.
Useful? React with 👍 / 👎.
| except ImportError: | ||
| librosa = PlaceholderModule("librosa") | ||
|
|
||
| import cv2 |
There was a problem hiding this comment.
vllm/benchmarks/datasets.py now imports cv2 at module import time, unlike other optional dependencies which are wrapped in try/except placeholders. Any environment that uses text-only benchmarks and does not have OpenCV installed will now hit an ImportError simply by importing vllm.benchmarks.datasets, even though video sampling might never be used. This tightens a new hard dependency that will break existing benchmark workflows unless OpenCV is installed everywhere.
Useful? React with 👍 / 👎.
| @pytest.mark.benchmark | ||
| def test_generate_synthetic_video_different_seeds(self): | ||
| """Test that different seeds produce different videos.""" | ||
| dataset1 = RandomMultiModalDataset(random_seed=123) | ||
| dataset2 = RandomMultiModalDataset(random_seed=456) | ||
|
|
||
| width, height, num_frames = 64, 48, 8 | ||
|
|
||
| video1 = dataset1.generate_synthetic_video(width, height, num_frames) |
There was a problem hiding this comment.
self fixture and fails to collect
The new test test_generate_synthetic_video_different_seeds is a module-level function but declares a self parameter. Pytest will treat self as a fixture and raise FixtureLookupError: no fixture named 'self' during collection, so the entire test module never runs and all subsequent assertions are skipped. Removing the self argument fixes test collection.
Useful? React with 👍 / 👎.
Allow benchmarking models using
random-mmdataset with video inputsPurpose
Test Plan
Test Result
Essential Elements of an Effective PR Description Checklist
supported_models.mdandexamplesfor a new model.