Skip to content

Conversation

DarkLight1337
Copy link
Member

@DarkLight1337 DarkLight1337 commented Aug 29, 2024

Currently, the placeholder tokens are silently truncated if they exceed the context length, causing confusing errors when later assigning multimodal features to the placeholder tokens inside the model. (e.g. #6176)

This PR avoids such problems by checking the length of the processed prompt beforehand.

Copy link

👋 Hi! Thank you for contributing to the vLLM project.
Just a reminder: PRs would not trigger full CI run by default. Instead, it would only run fastcheck CI which consists a small and essential subset of CI tests to quickly catch errors. You can run other CI tests on top of default ones by unblocking the steps in your fast-check build on Buildkite UI.

Once the PR is approved and ready to go, please make sure to run full CI as it is required to merge (or just use auto-merge).

To run full CI, you can do one of these:

  • Comment /ready on the PR
  • Add ready label to the PR
  • Enable auto-merge.

🚀

@DarkLight1337 DarkLight1337 added the ready ONLY add when PR is ready to merge/full CI is needed label Aug 29, 2024
Copy link
Member

@ywang96 ywang96 left a comment

Choose a reason for hiding this comment

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

Left a nit but otherwise LGTM!

Comment on lines +1967 to +1969
"number of text tokens plus multimodal tokens. For image "
"inputs, the number of image tokens depends on the number "
"of images, and possibly their aspect ratios as well.")
Copy link
Member

Choose a reason for hiding this comment

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

Nit: we can probably just generalize the wording here for all multimodal data items instead of calling out image in particular, but we can do this change in a later PR.

@simon-mo simon-mo enabled auto-merge (squash) August 30, 2024 00:48
@simon-mo simon-mo disabled auto-merge August 30, 2024 00:49
@simon-mo simon-mo merged commit 4abed65 into vllm-project:main Aug 30, 2024
46 of 48 checks passed
@DarkLight1337 DarkLight1337 deleted the check-mm-max-model-len branch August 30, 2024 00:49
Alvant pushed a commit to compressa-ai/vllm that referenced this pull request Oct 26, 2024
LeiWang1999 pushed a commit to LeiWang1999/vllm-bitblas that referenced this pull request Mar 26, 2025
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