Skip to content
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

Add openai embedding API #997

Merged
merged 1 commit into from
Aug 9, 2024
Merged

Add openai embedding API #997

merged 1 commit into from
Aug 9, 2024

Conversation

Ying1123
Copy link
Member

@Ying1123 Ying1123 commented Aug 9, 2024

@Ying1123 Ying1123 force-pushed the openai-embedding branch 2 times, most recently from 5cf0901 to c343e77 Compare August 9, 2024 08:45
@Ying1123 Ying1123 force-pushed the openai-embedding branch 2 times, most recently from 363da49 to 0807173 Compare August 9, 2024 08:56
@Ying1123 Ying1123 mentioned this pull request Aug 9, 2024
29 tasks
@yichuan520030910320
Copy link
Collaborator

LGTM

@yichuan520030910320
Copy link
Collaborator

btw, I have one small question, why switch the max_num_tokens from 1 to 0?

@Ying1123
Copy link
Member Author

Ying1123 commented Aug 9, 2024

btw, I have one small question, why switch the max_num_tokens from 1 to 0?

Did you mean switch from 0 to 1? This is because our default logic for generation model suppose it will generate 1 token after prefill. Although embedding models do not generate tokens, I added one dummy token to cheat with the shared part of the code.

@Ying1123 Ying1123 merged commit b16e856 into main Aug 9, 2024
4 checks passed
@Ying1123 Ying1123 deleted the openai-embedding branch August 9, 2024 18:19
@yichuan520030910320
Copy link
Collaborator

btw, I have one small question, why switch the max_num_tokens from 1 to 0?

Did you mean switch from 0 to 1? This is because our default logic for generation model suppose it will generate 1 token after prefill. Although embedding models do not generate tokens, I added one dummy token to cheat with the shared part of the code.

I see, thanks for answering

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