Skip to content

Remove tiktoken from logit bias to fix API logit bias#5391

Merged
oobabooga merged 27 commits intooobabooga:devfrom
Ph0rk0z:patch-3
Jan 29, 2024
Merged

Remove tiktoken from logit bias to fix API logit bias#5391
oobabooga merged 27 commits intooobabooga:devfrom
Ph0rk0z:patch-3

Conversation

@Ph0rk0z
Copy link
Copy Markdown
Contributor

@Ph0rk0z Ph0rk0z commented Jan 28, 2024

Checklist:

TikToken is not used in other parts of the file, they are commented out. The model parameter is not required for internal tokenization. The models tiktoken expects are only served by openAI. If there is another reason to keep this please please comment. Since the previous patch wasn't clean by skipping via exception, perhaps this is better. Otherwise we cannot use logit bias over the API at all and it is a broken feature.

fixes: #5232

oobabooga and others added 27 commits December 14, 2023 22:39
@Ph0rk0z Ph0rk0z changed the base branch from main to dev January 28, 2024 16:19
@Ph0rk0z
Copy link
Copy Markdown
Contributor Author

Ph0rk0z commented Jan 28, 2024

edit: target to dev not main

@oobabooga
Copy link
Copy Markdown
Owner

I think that removing tiktoken is a good thing for consistency. Have you made tests over API to validate that the logit bias is working as intended? If so the PR can be merged.

@Ph0rk0z
Copy link
Copy Markdown
Contributor Author

Ph0rk0z commented Jan 28, 2024

Yes.. I tested it first with silly tavern. Definitely can remove a token with negative and make the token the only reply with positive.

@oobabooga
Copy link
Copy Markdown
Owner

Looks good then

@oobabooga oobabooga merged commit 528318b into oobabooga:dev Jan 29, 2024
PoetOnTheRun pushed a commit to PoetOnTheRun/text-generation-webui that referenced this pull request Feb 22, 2024
@Ph0rk0z Ph0rk0z deleted the patch-3 branch May 12, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants