-
Notifications
You must be signed in to change notification settings - Fork 830
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
perf: Update OpenAI Embedding with latest embedding model #1938
Conversation
Hey @dciborow 👋! We use semantic commit messages to streamline the release process. Examples of commit messages with semantic prefixes:
To test your commit locally, please follow our guild on building from source. |
notebooks/features/cognitive_services/CognitiveServices - OpenAI Embedding.ipynb
Outdated
Show resolved
Hide resolved
@mhamilton723 , i deployed the 'text-embedding-ada-002' endpoint. This should be all ready for testing. |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Codecov Report
@@ Coverage Diff @@
## master #1938 +/- ##
==========================================
- Coverage 87.01% 86.98% -0.04%
==========================================
Files 305 305
Lines 15993 15993
Branches 839 839
==========================================
- Hits 13917 13911 -6
- Misses 2076 2082 +6 |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary by GPT-4
The changes in the code are as follows:
- Added a comment with a link to learn more about selecting which embedding model to choose.
- Changed the
deployment_name_embeddings
variable value from"text-search-ada-doc-001"
to"text-embedding-ada-002"
. - Removed the
deployment_name_embeddings_query
variable and its value"text-search-ada-query-001"
. - In the
embedding_query
code block, replaced.setDeploymentName(deployment_name_embeddings_query)
with.setDeploymentName(deployment_name_embeddings)
.
These changes update the deployment name for the embeddings and remove an unnecessary variable, simplifying the code.
Suggestions
No suggestions are needed as the changes in this PR are clear and straightforward.
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
Related Issues/PRs
Provide an example for the user of using the latest embedding model if it is available.
What changes are proposed in this pull request?
Briefly describe the changes included in this Pull Request.
How is this patch tested?
Does this PR change any dependencies?
Does this PR add a new feature? If so, have you added samples on website?
website/docs/documentation
folder.Make sure you choose the correct class
estimators/transformers
and namespace.DocTable
points to correct API link.yarn run start
to make sure the website renders correctly.<!--pytest-codeblocks:cont-->
before each python code blocks to enable auto-tests for python samples.WebsiteSamplesTests
job pass in the pipeline.