-
Notifications
You must be signed in to change notification settings - Fork 135
Add max_pixels option. #2094
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add max_pixels option. #2094
Conversation
scripts/start_gaudi_vllm_server.sh
Outdated
| --trust-remote-code \ | ||
| --seed 2025 \ | ||
| --distributed_executor_backend "${dist_backend}" \ | ||
| --mm_processor_kwargs "max_pixels=1003520" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this value be configurable by users and set 1003520 as default value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I change it to configurable, what option flag do you suggest I use?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I change it to configurable, what option flag do you suggest I use?
how about just reuse -e
cc @yangulei
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I update the PR and try to reuse -e option to set --mm-processor-kwargs.
It is troublesome to process "max_pixles" alone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you want to pass --mm_processor_kwargs "max_pixels=1003520" to the vllm server, just pass -e "--mm_processor_kwargs max_pixels=1003520" to the start_gaudi_vllm_server.sh. We don't need to change the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should better set a default value in script because user may doesn't know this options is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use -e opton, but with a default value for mm-processor-kwargs. How about this implementation?
808e207 to
4a27c63
Compare
When input attn_mask and seq_len is large, FusedSDPA fail. Because large input image doesn't use pad, attn_mask is set to None for large input image to avoid this error. Signed-off-by: Chen, Wenbin <wenbin.chen@intel.com>
4a27c63 to
af933e6
Compare
Add max_pixels option, so that vllm resize large images.