Skip to content

Conversation

thecoop
Copy link
Member

@thecoop thecoop commented Feb 12, 2025

Add comparisons of float[] queries with byte[] vectors

This fixes #117274


@Override
public float ipFloatByte(float[] q, byte[] d) {
return DefaultESVectorUtilSupport.ipFloatByteImpl(q, d);
Copy link
Member Author

Choose a reason for hiding this comment

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

Panama implementation can be added as a separate PR

Copy link
Member Author

Choose a reason for hiding this comment

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

See #123270

@thecoop
Copy link
Member Author

thecoop commented Feb 12, 2025

We could do this the other way too - add implementations of BinaryDenseVector.dotProduct(byte[]) and friends

@elasticsearchmachine
Copy link
Collaborator

Hi @thecoop, I've created a changelog YAML for you.

@thecoop thecoop marked this pull request as ready for review February 12, 2025 16:29
@thecoop thecoop requested a review from benwtrent February 12, 2025 16:29
@elasticsearchmachine elasticsearchmachine added the Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch label Feb 12, 2025
@elasticsearchmachine
Copy link
Collaborator

Pinging @elastic/es-search-relevance (Team:Search Relevance)

@benwtrent
Copy link
Member

I think this looks good as a first step.

Could you add some tests here: modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/145_dense_vector_byte_basic.yml

Obviously, a component will need to be added. But it will help increase our coverage for free.

@thecoop thecoop requested a review from a team as a code owner February 12, 2025 17:11
@thecoop thecoop requested a review from benwtrent February 21, 2025 13:01
@thecoop
Copy link
Member Author

thecoop commented Feb 21, 2025

More heterogenous methods can be introduced in later PRs using this infrastructure

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

This is great! I have one concern about cosine.

@thecoop thecoop force-pushed the float-byte-script-comparisons branch from 4374e6c to 1d99668 Compare February 27, 2025 16:35
Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

Almost there :). I think this latest change does a little bit too much work, its fine to keep the previous magnitude calculation as the individual component values do not change (we simply cast them).

Copy link
Member

@benwtrent benwtrent left a comment

Choose a reason for hiding this comment

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

:shipit: ❤️

@thecoop thecoop merged commit 82668b4 into elastic:main Mar 3, 2025
16 of 17 checks passed
@thecoop thecoop deleted the float-byte-script-comparisons branch March 3, 2025 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>enhancement :Search Relevance/Vectors Vector search Team:Search Relevance Meta label for the Search Relevance team in Elasticsearch v9.1.0

3 participants