[Voxtral] Add more tests #21010
[Voxtral] Add more tests #21010
Conversation
patrickvonplaten
requested review from
DarkLight1337,
ywang96,
robertgshaw2-redhat,
simon-mo and
aarnphm
as code owners
|
👋 Hi! Thank you for contributing to the vLLM project. 💬 Join our developer Slack at https://slack.vllm.ai to discuss your PR in #pr-reviews, coordinate on features in #feat- channels, or join special interest groups in #sig- channels. Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run Once the PR is approved and ready to go, your PR reviewer(s) can run CI to test the changes comprehensively before merging. To run CI, PR reviewers can either: Add 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request enables and adds tests for the Voxtral model. The changes are mostly in test files. I've found one high-severity issue in a test helper function that could lead to an IndexError due to an unsafe list access. I've provided a suggestion to fix it.
Signed-off-by: Patrick von Platen <[email protected]>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Patrick von Platen <[email protected]>
patrickvonplaten
force-pushed
the
add_vox_tests
branch
from
cb75e2c to
f4973f7
Compare
Signed-off-by: Patrick von Platen <[email protected]>
github-actions
bot
added
the
ready
| @@ -440,7 +440,7 @@ def check_available_online( | |||
| tokenizer="Isotr0py/Florence-2-tokenizer", # noqa: E501 | |||
| trust_remote_code=True), # noqa: E501 | |||
| "MllamaForConditionalGeneration": _HfExamplesInfo("meta-llama/Llama-3.2-11B-Vision-Instruct"), # noqa: E501 | |||
| "VoxtralForConditionalGeneration": _HfExamplesInfo("mistralai/Voxtral-Mini-3B-2507", is_available_online=False, tokenizer_mode="mistral"), # noqa: E501 | |||
| "VoxtralForConditionalGeneration": _HfExamplesInfo("mistralai/Voxtral-Mini-3B-2507", tokenizer_mode="mistral"), # noqa: E501 | |||
There was a problem hiding this comment.
Doesn't this also need --config_format mistral --load_format mistral?
There was a problem hiding this comment.
Yeah actually that'd be safer - good point. It doesn't need it if there are only "mistral" format weights, but as soon as we add transformers weights to the repo it probably breaks. Will open a quick fix
There was a problem hiding this comment.
Ah but can't set config_format or load_format - hmm should I add config_format and load_format then also to the _HfExamplesInfo object?
There was a problem hiding this comment.
Yeah but you need to also update a bunch of tests where HF_EXAMPLE_MODELS is being used to ensure the settings are being forwarded correctly to vLLM
There was a problem hiding this comment.
This test starts failing for everyone on CI, i.e. https://buildkite.com/vllm/ci/builds/24375#01981fb2-e37e-44e5-a313-b157a4ffd669/8532-9390, after https://huggingface.co/mistralai/Voxtral-Mini-3B-2507/commit/c63c83ed2e3bbb75da6f2bf67f9a499690c1e689#d2h-457069 was committed to HF today. It looks like a new transformers package is needed, but it's not public yet 4.54.0.dev0. Would there be any short-term mitigation to keep CI sane until we have that?
Signed-off-by: Patrick von Platen <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: Himanshu Jaju <[email protected]>
Signed-off-by: Patrick von Platen <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Signed-off-by: Patrick von Platen <[email protected]> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Signed-off-by: avigny <[email protected]>
Follow up of: #20970