Closed yinsong1986 closed 2 months ago
DarkLight1337
commented
2 months ago @xwjiang2010 is this related to sharding being disabled for vision tower? Personally I haven't run into this problem with tp=2 but it may become an issue with other tp values.
yinsong1986
commented
2 months ago Hi @DarkLight1337 ,
Thanks for the comments, for me, even tp=2 doesn't work, if you use image_features as the input.
DarkLight1337
commented
2 months ago Hi @DarkLight1337 ,
Thanks for the comments, for me, even tp=2 doesn't work, if you use
image_featuresas the input.
Just realized that the problem is specific to image_features. Do you have a specific use case for this type of input? We're planning to remove support for it in a future release and replace it with image embeddings (unlike image features, this refers to the output after the multimodal projector)
yinsong1986
commented
2 months ago Using image_features can bypass the limitation of only allowing 1 image for the input. In other words, we can pass the image_features for multiple images.
To fix this bug, my understanding is to change this line https://github.com/vllm-project/vllm/blob/v0.5.0.post1/vllm/model_executor/models/llava.py#L218 to sth like
device = torch.device('cuda' if torch.cuda.is_available() else 'cpu')
image_features = image_input["data"].to(device)Any idea? Thanks!
DarkLight1337
commented
2 months ago Using image_features can bypass the limitation of only allowing 1 image for the input. In other words, we can pass the image_features for multiple images.
To fix this bug, my understanding is to change this line https://github.com/vllm-project/vllm/blob/v0.5.0.post1/vllm/model_executor/models/llava.py#L218 to sth like
device = torch.device('cuda' if torch.cuda.is_available() else 'cpu') image_features = image_input["data"].to(device)Any idea? Thanks!
I doubt this would work since the error is raised before the model is even called. It seems to be related to how multi modal data is sent to the individual workers by the model executor.
xwjiang2010
commented
2 months ago Hi @DarkLight1337 @yinsong1986
The problem is also at play with pixel_values + tp > 1. The symptom is hidden though because this line of code moves the tensor to the right device.
Basically pixel_values is put on cuda:0 first and then moved to cuda:x to be processed by the vision tower there.
What should happen instead is pixel_values (or image_features for that matter) should be put on cuda:x from the very beginning (how tp works).
@DarkLight1337 since you are doing the input processing stuff, can you make sure that this bug is fixed for all the 3 models we support? Thank you!
For my future reference, I am using main branch at commit 67005a07bc0991211ba2acccb3e56c72a47f9def. Using head will give me shm errors. I believe that is introduced by the recent shm backend changes and is irrelevant for vlm testing.
xwjiang2010
commented
2 months ago @yinsong1986 Another note, as @DarkLight1337 mentioned, we are removing the support of image_features and pixel_values in an effort to make API more user friendly. Instead we focus support on PIL.Image.Image and embeddings. I believe that your use case will work similarly with embeddings.
DarkLight1337
commented
2 months ago What should happen instead is pixel_values (or image_features for that matter) should be put on cuda:x from the very beginning (how tp works).
@DarkLight1337 since you are doing the input processing stuff, can you make sure that this bug is fixed for all the 3 models we support? Thank you!
Where should I put this code? I thought the tensors were already moved to the correct device inside the model runner?
xwjiang2010
commented
2 months ago @DarkLight1337 yes, I would think model_runner already did that. But apparently that's not the case. Clearly a bug somewhere. I will spend 1-2 hours today to look into it.
ywang96
commented
2 months ago Hi @yinsong1986! We have discussed offline regarding this bug - there's some ongoing effort #5851 already on removing the support for image features, and eventually we will support image embeddings instead. The difference here is that image features still need to go through a multi-modal projector to generate the image embeddings to be consumed by the language model, and having a clear cut of whether or not the vision encoder path is needed in the forward pass is better in our opinion.
We will also investigate and fix the bug why TP didn't work, but this won't be done for image features but in a later PR when we add the support for image embeddings
yinsong1986
commented
2 months ago Thanks @DarkLight1337 @xwjiang2010 Look forward to the changes!
Also, FYI: I tested, as a work around to support image_features for tp>1, we can change this line https://github.com/vllm-project/vllm/blob/v0.5.0.post1/vllm/model_executor/models/llava.py#L218 to sth like
device = torch.device('cuda' if torch.cuda.is_available() else 'cpu')
image_features = image_input["data"].to(device)It could kind of run successfully with tp=8. Cheers!
lonngxiang
commented
2 months ago same run conmmad:
python -m vllm.entrypoints.openai.api_server --model /ai/LLaVA-NeXT --image-token-id 32000 --image-input-shape 1,3,336,336 --image-input-type pixel_values --image-feature-size 65856 --chat-template template_llava.jinja --host 192.1***--port 10860 --trust-remote-code --tensor-parallel-size 2 --disable-custom-all-reduce The fix will be included in the upcoming release. You can build from source (main branch) if you need the fix right now.
lonngxiang
commented
2 months ago I'm really looking forward to it and can't wait.
The fix will be included in the upcoming release. You can build from source (
mainbranch) if you need the fix right now.
Your current environment
🐛 Describe the bug
When following the example https://github.com/vllm-project/vllm/blob/v0.5.0.post1/examples/llava_example.py and enabled multiple GPUs as below:
and used
image_featuresas the argument of--type, and ranpython example/llava_example.py --type image_features, it would report errors like below: