-
Notifications
You must be signed in to change notification settings - Fork 530
[Refactor]Refactor sampler #2050
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
Conversation
|
@Pr0Wh1teGivee please help review. Thanks. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2050 +/- ##
==========================================
- Coverage 73.83% 73.80% -0.03%
==========================================
Files 96 96
Lines 10865 10899 +34
==========================================
+ Hits 8022 8044 +22
- Misses 2843 2855 +12
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
LGTM |
| probs = logits.softmax(dim=-1) | ||
| probs_sort, _ = probs.sort(dim=-1, descending=False) |
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.
Unrelated: it seems probs and probs_sort compution are ununsed if p is None and k is None, the logits can return directly.
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.
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
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
1 similar comment
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
| else: | ||
| from vllm.v1.sample.sampler import Sampler | ||
|
|
||
| self.sampler = Sampler() |
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.
Can it take effect directly by removing the flag?
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.
yes, I hope so. Let's have a fully test to ensure ascend sampler is stable enough. Then we can remove the flag
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 set the value to True to enable ascend sampler by default now.
b5c5975 to
bb4782c
Compare
bb4782c to
8206a55
Compare
Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
8206a55 to
81d6a67
Compare
Refactor Sampler implementation from patch way to inherit from vLLM Sampler interface. Next step: Make the op `TopKTopPSampler` in vLLM support custom ops register mechanism - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@61a6905 Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
Refactor Sampler implementation from patch way to inherit from vLLM Sampler interface. Next step: Make the op `TopKTopPSampler` in vLLM support custom ops register mechanism - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@61a6905 Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com> Signed-off-by: weijinqian_v1 <weijinqian@huawei.com>
Refactor Sampler implementation from patch way to inherit from vLLM Sampler interface. Next step: Make the op `TopKTopPSampler` in vLLM support custom ops register mechanism - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@61a6905 Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
Refactor Sampler implementation from patch way to inherit from vLLM Sampler interface. Next step: Make the op `TopKTopPSampler` in vLLM support custom ops register mechanism - vLLM version: v0.10.0 - vLLM main: vllm-project/vllm@61a6905 Signed-off-by: wangxiyuan <wangxiyuan1007@gmail.com>
Refactor Sampler implementation from patch way to inherit from vLLM Sampler interface.
Next step: Make the op
TopKTopPSamplerin vLLM support custom ops register mechanism