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 embedding mapper for text-embedding-ada-002 model #33

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fakechris
Copy link

No description provided.

Copy link

@Wetiqe Wetiqe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about doing the same thing. This works fine for the embedding model. This will be useful for using packages such as llama-index

@haibbo
Copy link
Owner

haibbo commented Jun 1, 2023

@fakechris
Thanks for your PR, It‘s useful for developer.
Here are some suggestions:
1, Add a function to handler /v1/moderations, e.g. handleModerations

  • this change will make route part clear

2, Remove text-embedding-ada-002 mapper

  • Most users are not developers, and adding this mapper would make it more difficult for them to configure.
  • However, it's easy for developers to add a model mapper they are familiar with.

@lubuwei001
Copy link

lubuwei001 commented Aug 20, 2023

@fakechris Thanks for your PR, It‘s useful for developer. Here are some suggestions: 1, Add a function to handler /v1/moderations, e.g. handleModerations

  • this change will make route part clear

2, Remove text-embedding-ada-002 mapper

  • Most users are not developers, and adding this mapper would make it more difficult for them to configure.
  • However, it's easy for developers to add a model mapper they are familiar with.

When using text analysis services such as Zotero GPT, there is still a need to add an embedding mapper for the text-embedding-ada-002 model. Thank you for the code @fakechris , it's very helpful.

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.

None yet

4 participants