You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Top-p/Top-k Sampling Params handling in VLLM v1 #69
Just for notes: there are some good discussions for Top-p/Top-k support in #766.
raw logprobs are fine for now (same as v1). But are actually incorrect for top-k, top-p sampling (even though it appears as though it would work correctly). Both the logprobs from vllm and the training framework are incorrect since they haven't been processed, which introduces error. (I believe the claim from #773 that v1 is fine is incorrect)
raw logprobs are fine for now (same as v1). But are actually incorrect for top-k, top-p sampling (even though it appears as though it would work correctly). Both the logprobs from vllm and the training framework are incorrect since they haven't been processed, which introduces error. (I believe the claim from #773 that v1 is fine is incorrect)
use vllm post-processed logprobs and apply the processing to just megatron.
I'm leaning strongly towards the latter as it leaves less room for us to make mistakes and not notice. We should track this in #69.
I feel so too, although raw_logprobs is ok for temperature, the logits when using top-k/top-p will become incorrect if not actually applying top-k/top-p on them. And agreed with the latter one.
I think we can get this PR with raw_logprobs merged first. Then with @Dazz993 's help on #773 and #774, to support it at training side (both dtensor and megatron) and change raw_logprobs to processed_logprobs that time.
Activity
terrykong commentedon May 8, 2025
#316 related
wangshangsam commentedon Jul 2, 2025
Assigning to @Dazz993
github-actions commentedon Aug 4, 2025
This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.
zhandaz commentedon Aug 5, 2025
Just for notes: there are some good discussions for Top-p/Top-k support in #766.
raw logprobs are fine for now (same as v1). But are actually incorrect for top-k, top-p sampling (even though it appears as though it would work correctly). Both the logprobs from vllm and the training framework are incorrect since they haven't been processed, which introduces error. (I believe the claim from #773 that v1 is fine is incorrect)
We have 2 options for how to mitigate this:
TP==1#773 on both vllm raw_logprobs and megatron trainingI'm leaning strongly towards the latter as it leaves less room for us to make mistakes and not notice. We should track this in #69.
Originally posted by @SahilJain314 in #766 (comment)
I feel so too, although
raw_logprobsis ok for temperature, the logits when using top-k/top-p will become incorrect if not actually applying top-k/top-p on them. And agreed with the latter one.I think we can get this PR with
raw_logprobsmerged first. Then with @Dazz993 's help on #773 and #774, to support it at training side (both dtensor and megatron) and changeraw_logprobstoprocessed_logprobsthat time.Originally posted by @YUki-666 in #766 (comment)
zhandaz commentedon Aug 6, 2025
Please see #773 for up-to-date discussions.
fix(algorithms): guard ZeroDivisionError in print_performance_metrics
yuki-97 commentedon May 25, 2026
completed through #2053