-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
[core] add bucket padding to tpu_model_runner #14995
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
[core] add bucket padding to tpu_model_runner #14995
Conversation
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
👋 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 🚀 |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
@Chenyaaang Thanks for your contribution, left some comments above! @robertgshaw2-redhat I know there's another configuration option |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
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.
Good idea!
Please fix the merge conflict |
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.
looks good! Also in favor of unifying with cudagraph_capture_sizes
This pull request has merge conflicts that must be resolved before it can be |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
Head branch was pushed to by a user without write access
I think the param is useful for online serving, by exposing this parameter, users can adjust the gap based on the model size. My understanding is for example the gap is small then it will lead to a long pre-compile time, that's something customer can adjust. |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
This PR seems to have broken CUDA |
@Chenyaaang thanks for reducing the padding gap, this is useful. Could you please address @robertgshaw2-redhat comment and fix the build (so we can merge it). Thanks! |
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
I've fixed the build and replied to @robertgshaw2-redhat's comment. |
Signed-off-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
tests/tpu/test_compilation.py
Outdated
# Check we have 4 compiled codes | ||
assert len(compiled_codes) == 4 | ||
# Check we have 3 compiled codes | ||
assert len(compiled_codes) == 3 |
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.
@Chenyaaang Thanks for looking into this! Could we branch out for v0 and v1? In v0 it should be 4 compiled 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.
Done, thanks!
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
a725d2b
to
7d92244
Compare
33a8bbc
to
7535bd9
Compare
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com> Signed-off-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Co-authored-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Signed-off-by: Wes Medford <wryanmedford@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com> Signed-off-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Co-authored-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com> Signed-off-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Co-authored-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Signed-off-by: Louis Ulmer <ulmerlouis@gmail.com>
Signed-off-by: Chenyaaang <llccyy1212@gmail.com> Signed-off-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com> Co-authored-by: rshaw@neuralmagic.com <robertgshaw2@gmail.com>
Add bucket padding to tpu, instead of padding to the power of 2, if num_token < bucket_padding_gap, pad to the nearest power of 2, if num_token > bucket_padding_gap, the padding size is increased by bucket_padding_gap.
For example, bucket_padding_gap = 64, max_num_batch_tokens = 512, then the paddings will be 16, 32, 64, 128, 192, 256, 320, 384, 448, 512. This helps reduce the computation cost for large num_tokens, e.g. num_tokens = 300, instead of padding to 512, now pad to 320.
FIX #14581