Add access to dense_vector values #71313

mayya-sharipova · 2021-04-05T21:21:56Z

Allow direct access to a dense_vector' values in script
through the following functions:

getVectorValue – returns a vector's value as an array of floats
getVectorMagnitude – returns a vector's magnitude

Closes #51964

Allow direct access to a dense_vector' values in script through the following functions: - getVectorValue – returns a vector's value as an array of floats - getVectorMagnitude – returns a vector's magnitude Closes elastic#51964

elasticmachine · 2021-04-05T21:22:44Z

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

...lugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/VectorEncoderDecoder.java

…ator

jtibshirani

This seems like a helpful addition! I left a few high-level comments to start.

One overall comment: the original issue mentions exposing an iterator, whereas we return a decoded array here. I don't have major concerns with this, but wanted to highlight the difference.

...ugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/20_dense_vector_special_cases.yml

...lugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/VectorEncoderDecoder.java

jtibshirani · 2021-04-06T18:59:05Z

docs/reference/vectors/vector-functions.asciidoc

+
+- `float[] getVectorValue()` – returns a vector's value as an array of floats
+
+- `float getVectorMagnitude()` – returns a vector's magnitude (available for


Could we implement a slow version of getVectorMagnitude for vectors before 7.5? This seems easy and would make the API simpler. (Then we also might be able to use DenseVectorScriptDocValues#getVectorMagnitude to remove some logic inside cosineSimilarity that requires direct access to the index version!)

@jtibshirani Thanks for the feedback. I was also thinking the same – to implement a slower version of getVectorMagnitude, but this would require decoding the whole vector. If a user is already using vectorValue in their script and decoding a vector, using magnitude would mean decoding this vector the second time. So it would be faster for a user to implement magnitude function themselves since they would already have the decoded vector available. WDYT?

To me it's the right trade-off for a simple API. First, it will only be slow for vectors indexed before 7.5, which was before they were even GA. It also seems okay that it's slow, users can easily work around it. Maybe we could just write a short note in the docs about the pre-7.5 behavior so that users are aware.

docs/reference/vectors/vector-functions.asciidoc

-decodeVectorMagnitude go back to Buffer -correct documentation to use doc access methods for vector values and magnitude instead of get functions - rename getVectorMagnitude to getMagnitude - remove unnecessary yml tests

mayya-sharipova · 2021-04-08T20:54:49Z

@jtibshirani Thanks for your feedback. I've tried to address your comments in 388b1f8.

One overall comment: the original issue mentions exposing an iterator, whereas we return a decoded array here. I don't have major concerns with this, but wanted to highlight the difference.

Indeed, we don't return an iterator here. Thanks for highlighting this difference. But considering that the primary use-case for vectors is to process its all elements, and I've never encountered the need for iterators, I guess this should be fine to return all vector elements at once.

docs/reference/mapping/types/dense-vector.asciidoc

jtibshirani · 2021-04-12T22:46:21Z

...ugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/30_dense_vector_script_access.yml

+          - '{}'
+
+  # check getVectorValue() API
+  - do:


This test coverage looks good! Since unit tests are generally easier to work with than REST tests, I wondered if there was a way perform some of the same checks as unit tests?

@jtibshirani Thanks for the feedback. I could not find any good examples of doing unit tests with script doc values. Unit tests that we have they mock scripts contexts and mock what script returns, which kind of defeats the purpose of testing what getVectorValue() and getMagnitude() returns.

I am happy to redesign tests as unit tests, if you know any examples I can follow.

It seems like there are some simple cases where we just want to check getVectorValue and getMagnitude return the right value or error appropriately. These could be covered in a test like DenseVectorScriptDocValuesTests. A similar test would be ScriptDocValuesGeoPointsTests.

jtibshirani · 2021-04-12T22:51:50Z

docs/reference/vectors/vector-functions.asciidoc

+
+- `float[] getVectorValue()` – returns a vector's value as an array of floats
+
+- `float getVectorMagnitude()` – returns a vector's magnitude (available for


To me it's the right trade-off for a simple API. First, it will only be slow for vectors indexed before 7.5, which was before they were even GA. It also seems okay that it's slow, users can easily work around it. Maybe we could just write a short note in the docs about the pre-7.5 behavior so that users are aware.

...ugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/30_dense_vector_script_access.yml

mayya-sharipova · 2021-04-14T12:39:17Z

@jtibshirani Thanks for another round of review. I've tried to address them in 51f509b

…ator

x-pack/plugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/query/ScoreScriptUtils.java

jtibshirani · 2021-04-14T17:34:42Z

...ugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/30_dense_vector_script_access.yml

+          - '{}'
+
+  # check getVectorValue() API
+  - do:


It seems like there are some simple cases where we just want to check getVectorValue and getMagnitude return the right value or error appropriately. These could be covered in a test like DenseVectorScriptDocValuesTests. A similar test would be ScriptDocValuesGeoPointsTests.

docs/reference/vectors/vector-functions.asciidoc

…ator

mayya-sharipova · 2021-04-14T22:57:18Z

@jtibshirani Thanks for another round of review. I've tried to address your feedback in a66f755

About DenseVectorScriptDocValuesTests, that's a great feedback.
I've added this test class to test basic functionalities of DenseVectorScriptDocValues.

I still kept 30_dense_vector_script_access.yml for a more involved test case using an array of query vectors to test to original problem reported by a user in #70437

jtibshirani

Looks good to me.

…ator

mayya-sharipova · 2021-04-15T18:21:12Z

@elasticmachine test this please

mayya-sharipova · 2021-04-15T18:22:29Z

@elasticmachine test this please

mayya-sharipova · 2021-04-16T19:05:26Z

@elasticmachine run elasticsearch-ci/bwc

…ator

mayya-sharipova · 2021-04-18T16:28:21Z

@elasticmachine run elasticsearch-ci/2

mayya-sharipova · 2021-04-18T18:21:09Z

@elasticmachine run elasticsearch-ci/2

mayya-sharipova · 2021-04-19T10:47:46Z

@elasticmachine update branch

Allow direct access to a dense_vector' values in script through the following functions: - getVectorValue – returns a vector's value as an array of floats - getMagnitude – returns a vector's magnitude Closes elastic#51964 Backport for elastic#71313

Allow direct access to a dense_vector' values in script through the following functions: - getVectorValue – returns a vector's value as an array of floats - getMagnitude – returns a vector's magnitude Closes #51964 Backport for #71313

Add access to dense_vector values

0857f54

Allow direct access to a dense_vector' values in script through the following functions: - getVectorValue – returns a vector's value as an array of floats - getVectorMagnitude – returns a vector's magnitude Closes elastic#51964

mayya-sharipova added >enhancement :Search Relevance/Ranking Scoring, rescoring, rank evaluation. labels Apr 5, 2021

elasticmachine added the Team:Search Meta label for search team label Apr 5, 2021

mayya-sharipova added v7.13.0 v8.0.0 labels Apr 5, 2021

mayya-sharipova requested a review from jtibshirani April 5, 2021 21:22

Small corrections

8d2e83a

mayya-sharipova commented Apr 6, 2021

View reviewed changes

...lugin/vectors/src/main/java/org/elasticsearch/xpack/vectors/mapper/VectorEncoderDecoder.java Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

d9dfdb1

…ator

jtibshirani reviewed Apr 6, 2021

View reviewed changes

Address Julie's feedback

388b1f8

-decodeVectorMagnitude go back to Buffer -correct documentation to use doc access methods for vector values and magnitude instead of get functions - rename getVectorMagnitude to getMagnitude - remove unnecessary yml tests

jtibshirani reviewed Apr 12, 2021

View reviewed changes

...ugin/src/yamlRestTest/resources/rest-api-spec/test/vectors/30_dense_vector_script_access.yml Outdated Show resolved Hide resolved

Address Julie's feedback 2

51f509b

mayya-sharipova force-pushed the dense-vector-iterator branch from 34d961b to 51f509b Compare April 14, 2021 13:31

mayya-sharipova added 2 commits April 14, 2021 09:38

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

6d7bd66

…ator

Address style check

d8edd03

jtibshirani reviewed Apr 14, 2021

View reviewed changes

docs/reference/vectors/vector-functions.asciidoc Outdated Show resolved Hide resolved

mayya-sharipova added 2 commits April 14, 2021 18:45

Address Julie's feedback 3

a66f755

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

503fdbc

…ator

jtibshirani approved these changes Apr 14, 2021

View reviewed changes

mayya-sharipova added 2 commits April 15, 2021 08:27

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

32a4165

…ator

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

c0fa8a9

…ator

Merge remote-tracking branch 'upstream/master' into dense-vector-iter…

8a6d6b8

…ator

Merge branch 'master' into dense-vector-iterator

116cf96

mayya-sharipova merged commit 853e68d into elastic:master Apr 19, 2021

mayya-sharipova deleted the dense-vector-iterator branch April 19, 2021 12:02

mayya-sharipova mentioned this pull request Apr 19, 2021

Add access to dense_vector values #71847

Merged

coreation mentioned this pull request Apr 29, 2021

Undocumented behaviour / Bug: ElasticSearch Painless - (Vector) Functions in (for) loops #70437

Closed

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

jtibshirani added :Search Relevance/Vectors Vector search and removed :Search Relevance/Ranking Scoring, rescoring, rank evaluation. labels Jul 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add access to dense_vector values #71313

Add access to dense_vector values #71313

mayya-sharipova commented Apr 5, 2021

elasticmachine commented Apr 5, 2021

jtibshirani left a comment

jtibshirani Apr 6, 2021

mayya-sharipova Apr 8, 2021 •

edited

Loading

jtibshirani Apr 12, 2021

mayya-sharipova commented Apr 8, 2021 •

edited

Loading

jtibshirani Apr 12, 2021

mayya-sharipova Apr 14, 2021

jtibshirani Apr 14, 2021

jtibshirani Apr 12, 2021

mayya-sharipova commented Apr 14, 2021 •

edited

Loading

jtibshirani Apr 14, 2021

mayya-sharipova commented Apr 14, 2021 •

edited

Loading

jtibshirani left a comment

mayya-sharipova commented Apr 15, 2021 •

edited

Loading

mayya-sharipova commented Apr 15, 2021

mayya-sharipova commented Apr 16, 2021

mayya-sharipova commented Apr 18, 2021

mayya-sharipova commented Apr 18, 2021

mayya-sharipova commented Apr 19, 2021


		- `float[] getVectorValue()` – returns a vector's value as an array of floats

		- `float getVectorMagnitude()` – returns a vector's magnitude (available for

Add access to dense_vector values #71313

Add access to dense_vector values #71313

Conversation

mayya-sharipova commented Apr 5, 2021

elasticmachine commented Apr 5, 2021

jtibshirani left a comment

Choose a reason for hiding this comment

jtibshirani Apr 6, 2021

Choose a reason for hiding this comment

mayya-sharipova Apr 8, 2021 • edited Loading

Choose a reason for hiding this comment

jtibshirani Apr 12, 2021

Choose a reason for hiding this comment

mayya-sharipova commented Apr 8, 2021 • edited Loading

jtibshirani Apr 12, 2021

Choose a reason for hiding this comment

mayya-sharipova Apr 14, 2021

Choose a reason for hiding this comment

jtibshirani Apr 14, 2021

Choose a reason for hiding this comment

jtibshirani Apr 12, 2021

Choose a reason for hiding this comment

mayya-sharipova commented Apr 14, 2021 • edited Loading

jtibshirani Apr 14, 2021

Choose a reason for hiding this comment

mayya-sharipova commented Apr 14, 2021 • edited Loading

jtibshirani left a comment

Choose a reason for hiding this comment

mayya-sharipova commented Apr 15, 2021 • edited Loading

mayya-sharipova commented Apr 15, 2021

mayya-sharipova commented Apr 16, 2021

mayya-sharipova commented Apr 18, 2021

mayya-sharipova commented Apr 18, 2021

mayya-sharipova commented Apr 19, 2021

mayya-sharipova Apr 8, 2021 •

edited

Loading

mayya-sharipova commented Apr 8, 2021 •

edited

Loading

mayya-sharipova commented Apr 14, 2021 •

edited

Loading

mayya-sharipova commented Apr 14, 2021 •

edited

Loading

mayya-sharipova commented Apr 15, 2021 •

edited

Loading