Skip to content

Conversation

ZJY0516
Copy link
Contributor

@ZJY0516 ZJY0516 commented Aug 25, 2025

Purpose

  1. make VoxtralProcessorAdapter return BatchFeature
(VllmWorker TP1 pid=13914) WARNING 08-25 19:00:18 [registry.py:183] VoxtralProcessorAdapter did not return `BatchFeature`. Make sure to match the behaviour of `ProcessorMixin` when implementing custom processors.
  1. pixtral: ditto

  2. pass a special token policy explicitly for self._special_token_policy is deprecated.

# mistral_common.tokens.tokenizers.tekken
if special_token_policy is None:
            warnings.warn(
                (
                    f"Using the tokenizer's special token policy ({self._special_token_policy}) is deprecated. "
                    "It will be removed in 1.10.0. "
                    "Please pass a special token policy explicitly. "
                    "Future default will be SpecialTokenPolicy.IGNORE."
                ),
                FutureWarning,
            )

Test Plan

vllm serve /data/datasets/models-hf/Voxtral-Small-24B-2507/ --tokenizer_mode mistral --config_format mistral --load_format mistral --tensor-parallel-size 2 --tool-call-parser mistral --enable-auto-tool-choice --served-model-name Voxtral

Test Result

(Optional) Documentation Update


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

Signed-off-by: zjy0516 <[email protected]>
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request effectively addresses two warnings related to the Mistral model. The first change correctly modifies VoxtralProcessorAdapter to return a BatchFeature object, aligning with the expected behavior of ProcessorMixin. The second set of changes addresses a deprecation warning by explicitly passing the special_token_policy to the underlying tokenizer methods, which is a good practice for future compatibility. The implementation looks solid and I don't see any critical or high-severity issues.

@DarkLight1337
Copy link
Member

cc @patrickvonplaten can you check this?

Copy link
Collaborator

@patrickvonplaten patrickvonplaten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactor overall - think we need to address some minor parts but looking good otherwise

Co-authored-by: Patrick von Platen <[email protected]>
Signed-off-by: Jiangyun Zhu <[email protected]>
@ZJY0516
Copy link
Contributor Author

ZJY0516 commented Aug 27, 2025

Thanks for your suggestion

Signed-off-by: zjy0516 <[email protected]>
Signed-off-by: zjy0516 <[email protected]>
@ZJY0516
Copy link
Contributor Author

ZJY0516 commented Aug 27, 2025

I think it' ready to merge. @DarkLight1337

@ZJY0516 ZJY0516 requested a review from DarkLight1337 August 29, 2025 03:47
@DarkLight1337 DarkLight1337 enabled auto-merge (squash) August 29, 2025 05:03
@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 29, 2025
@DarkLight1337 DarkLight1337 merged commit 885ca6d into vllm-project:main Aug 29, 2025
49 checks passed
@ZJY0516 ZJY0516 deleted the fix-mistral branch September 4, 2025 06:47
eicherseiji pushed a commit to eicherseiji/vllm that referenced this pull request Sep 9, 2025
Signed-off-by: zjy0516 <[email protected]>
Signed-off-by: Jiangyun Zhu <[email protected]>
Co-authored-by: Patrick von Platen <[email protected]>
FeiDaLI pushed a commit to FeiDaLI/vllm that referenced this pull request Sep 25, 2025
Signed-off-by: zjy0516 <[email protected]>
Signed-off-by: Jiangyun Zhu <[email protected]>
Co-authored-by: Patrick von Platen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready ONLY add when PR is ready to merge/full CI is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants