server : use common_token_to_piece instead of common_detokenize #11740
Conversation
|
Also, a clue is to look around the |
danbev
changed the title
There was a problem hiding this comment.
I can't comment on the code, but I've tested this patch and it builds successfully + resolves the issue #11728 . Thank you!
danbev
force-pushed
the
server-logprobs-11728
branch
from
456dabf to
cc1fd2f
Compare
This commit replaces the call to common_detokenize with common_token_to_piece in the populate_token_probs. The motivation for this change is to avoid an issue where common_detokenize would remove the word boundary character for tokens, which caused a regression in the server generated token probabilities. Resolves: ggerganov#11728
danbev
force-pushed
the
server-logprobs-11728
branch
from
cc1fd2f to
b70fd3a
Compare
|
IIRC there is another place having the same logic (near the speculative decode logic), that should be fixed too |
Use common_token_to_piece for post_sampling_probs as well.
I've taken a look but could not find this in/near the speculative decoding. I did find this TODO though. I noticed that this same logic is part of the post_sampling_probs and have updated this in 5deee0a. |
danbev
changed the title
…ganov#11740) * server : use common_token_to_piece instead of common_detokenize This commit replaces the call to common_detokenize with common_token_to_piece in the populate_token_probs. The motivation for this change is to avoid an issue where common_detokenize would remove the word boundary character for tokens, which caused a regression in the server generated token probabilities. Resolves: ggerganov#11728 * squash! server : use common_token_to_piece instead of common_detokenize Use common_token_to_piece for post_sampling_probs as well.
TThis commit replaces the call to
common_detokenizewithcommon_token_to_piecein the populate_token_probs.The motivation for this change is to avoid an issue where
common_detokenize would remove the word boundary character for tokens,
which caused a regression in the server generated token probabilities.
Resolves: #11728