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

[ML] Move byte embedding results to return results in text_embedding_bytes field #105290

Merged

Conversation

jonathan-buttner
Copy link
Contributor

@jonathan-buttner jonathan-buttner commented Feb 8, 2024

This PR came about because the the clients code is unable to deduce the correct response type in the current implementation of TextEmbeddingResults and TextEmbeddingByteResults being written to the same field text_embedding. After chatting with the clients team they suggested we have the byte results be written to a new field text_embedding_bytes to mitigate this.

This is a breaking change for cohere but hopefully not many people are using it yet since it hasn't been released.

Specification PR: elastic/elasticsearch-specification#2411

New response format

{
  "text_embedding_bytes": [
    {
      "embedding": [
        51,
        24,
        -22,
    },
   ...
  ]
}

@jonathan-buttner jonathan-buttner added >non-issue :ml Machine learning Team:ML Meta label for the ML team v8.13.0 labels Feb 8, 2024
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/ml-core (Team:ML)

Copy link
Member

@maxhniebergall maxhniebergall left a comment

Choose a reason for hiding this comment

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

LGTM

@jonathan-buttner jonathan-buttner merged commit 563c3e6 into elastic:main Feb 8, 2024
14 checks passed
@jonathan-buttner jonathan-buttner deleted the ml-inference-byte-result branch February 8, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:ml Machine learning >non-issue Team:ML Meta label for the ML team v8.13.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants