Ability to compute vector similarity scores with DoubleValuesSource #12548

shubhamvishu · 2023-09-11T02:03:48Z

Description

This PR addresses the issue #12394. It adds an API similarityToQueryVector to DoubleValuesSource to compute vector similarity scores between the query vector and the KnnByteVectorField/KnnFloatVectorField for documents using the 2 new DVS implementations (ByteVectorSimilarityValuesSource for byte vectors and FloatVectorSimilarityValuesSource for float vectors). Below are the method signatures added to DVS in this PR:

DoubleValues similarityToQueryVector(LeafReaderContext ctx, float[] queryVector, String vectorField) (uses ByteVectorSimilarityValuesSource)
DoubleValues similarityToQueryVector(LeafReaderContext ctx, byte[] queryVector, String vectorField) (uses FloatVectorSimilarityValuesSource)

Closes #12394

lucene/core/src/java/org/apache/lucene/search/ByteVectorSimilarityValuesSource.java

stefanvodita · 2023-09-30T12:02:06Z

lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java

+   */
+  public static DoubleValues similarityToQueryVector(
+      LeafReaderContext ctx, byte[] queryVector, String vectorField) throws IOException {
+    assert ctx.reader().getFieldInfos().fieldInfo(vectorField).getVectorEncoding()


What do you think of throwing an exception even when assertions are not enabled?

Makes sense! But I wonder if thats the case in many places in codebase?. I think we should rather throw an IAE in this case.

stefanvodita · 2023-09-30T12:03:42Z

lucene/core/src/java/org/apache/lucene/search/FloatVectorSimilarityValuesSource.java

+ * A {@link DoubleValuesSource} which computes the vector similarity scores between the query vector
+ * and the {@link org.apache.lucene.document.KnnFloatVectorField} for documents.
+ */
+class FloatVectorSimilarityValuesSource extends DoubleValuesSource {


This and BytesVectorSimilarityValuesSource look very similar. Should they inherit from a VectorSimilarityValuesSource to ensure we can add common functionality in the future?

There are some common bits. I have moved those to VectorSimilarityValuesSource now. Let me know if this looks more better or cleaner?

Great! I think this is the right approach.

lucene/core/src/test/org/apache/lucene/search/TestVectorSimilarityValuesSource.java

… comments

stefanvodita

Thank you for addressing the comments. The new version looks good!

benwtrent

The API looks OK to me.

shubhamvishu · 2023-10-12T14:11:58Z

Thanks for reviewing!

@benwtrent I have added a CHANGES.txt entry now, could you help me merging this if all looks good?

benwtrent · 2023-10-12T14:17:59Z

@shubhamvishu running CI :). I will see about merging and such soon.

msokolov

Looks great, thank you! I left some minor comments. Mostly I think we should move the static methods to a vector-specific file

msokolov · 2023-10-12T15:42:30Z

lucene/core/src/java/org/apache/lucene/search/ByteVectorSimilarityValuesSource.java

+import org.apache.lucene.index.VectorSimilarityFunction;
+
+/**
+ * A {@link DoubleValuesSource} which computes the vector similarity scores between the query vector


nit: my English profs always said to use "that" in place of "which" when possible

msokolov · 2023-10-12T15:46:02Z

lucene/core/src/java/org/apache/lucene/search/DoubleValuesSource.java

+   * @return DoubleValues instance
+   * @throws IOException if an {@link IOException} occurs
+   */
+  public static DoubleValues similarityToQueryVector(


Let's move these to VectorSimilaritySource? This file seem to be a dumping ground for all the things; let's not pile on.

Sure. I see different DVS use cases implemented as static classes here making this less readable. How about we move those static classes and functions(like you pointed) into a new class DoubleValuesSourceUtil maybe(only thing those classes are private currently)?. I'll open a followup PR to address that.

well I don't think we should rename all the existing static methods here - that would just cause confusion. Maybe that's not what you were saying?

Yes I didn't mean to move all the static methods here(sorry for the confusion). My idea was along your suggestion, should we also move the private static classes that are in this class(a lot in number, example - one, two and more) which is bloating this class out into another pkg-private class lets say DoubleValuesSourceUtil (the only implication being we would make these private classes pkg-private which is fine I think?). This way we could only keep the required API methods in DVS class and move out the rest stuff making this lean and readable. If that makes sense I'll put up these changes too with the followup CR(like below). Let me know what do you think?

Here is an short example commit I made to explain what I was saying - shubhamvishu@604f1fd

msokolov · 2023-10-12T15:48:31Z

oh I missed that @benwtrent had approved -- it's OK w/me to merge as-is, just had some idea about organizing the static methods into a different class...

shubhamvishu · 2023-10-12T15:51:11Z

oh I missed that @benwtrent had approved -- it's OK w/me to merge as-is, just had some idea about organizing the static methods into a different class...

Thanks @msokolov ! I'll open up a follow up PR to address your ideas if that sounds good?

benwtrent · 2023-10-12T17:34:15Z

These are good ideas @msokolov , @shubhamvishu a follow up PR is most welcome.

…12548) ### Description This PR addresses the issue #12394. It adds an API **`similarityToQueryVector`** to `DoubleValuesSource` to compute vector similarity scores between the query vector and the `KnnByteVectorField`/`KnnFloatVectorField` for documents using the 2 new DVS implementations (`ByteVectorSimilarityValuesSource` for byte vectors and `FloatVectorSimilarityValuesSource` for float vectors). Below are the method signatures added to DVS in this PR: - `DoubleValues similarityToQueryVector(LeafReaderContext ctx, float[] queryVector, String vectorField)` *(uses ByteVectorSimilarityValuesSource)* - `DoubleValues similarityToQueryVector(LeafReaderContext ctx, byte[] queryVector, String vectorField)` *(uses FloatVectorSimilarityValuesSource)* Closes #12394

shubhamvishu · 2023-10-13T12:22:31Z

@msokolov @benwtrent Raised a followup PR #12671 for this and also moved some other relevant static classes etc out of DVS(it now looks much better to me). Thanks!

shubhamvishu changed the title ~~Add API to compute vector similarity in DoubleValuesSource~~ Ability to compute vector similarity scores in DoubleValuesSource Sep 11, 2023

shubhamvishu changed the title ~~Ability to compute vector similarity scores in DoubleValuesSource~~ Ability to compute vector similarity scores with DoubleValuesSource Sep 11, 2023

shubhamvishu mentioned this pull request Sep 26, 2023

Add the ability to compute vector similarity scores with the new ValuesSource API #12394

Closed

stefanvodita reviewed Sep 30, 2023

View reviewed changes

shubhamvishu added 2 commits October 1, 2023 00:43

Add API to compute vector similarity in DoubleValuesSource

70324e3

Moved common code to VectorSimilarityValuesSource and addressed other…

97ff777

… comments

shubhamvishu force-pushed the knn-values-source branch from a7a19b5 to 97ff777 Compare October 1, 2023 10:24

shubhamvishu requested a review from stefanvodita October 1, 2023 10:25

stefanvodita approved these changes Oct 5, 2023

View reviewed changes

benwtrent approved these changes Oct 11, 2023

View reviewed changes

Changes.txt entry

910daf3

msokolov requested changes Oct 12, 2023

View reviewed changes

msokolov approved these changes Oct 12, 2023

View reviewed changes

benwtrent merged commit 52dfe50 into apache:main Oct 12, 2023
4 checks passed

shubhamvishu mentioned this pull request Oct 13, 2023

Move private static classes or functions out of DoubleValuesSource #12671

Closed

samuel-oci mentioned this pull request Nov 29, 2023

Knn example msfroh/lucene-university#1

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ability to compute vector similarity scores with DoubleValuesSource #12548

Ability to compute vector similarity scores with DoubleValuesSource #12548

shubhamvishu commented Sep 11, 2023 •

edited

Loading

stefanvodita Sep 30, 2023

shubhamvishu Oct 1, 2023

stefanvodita Sep 30, 2023

shubhamvishu Oct 1, 2023

stefanvodita Oct 5, 2023

stefanvodita left a comment

benwtrent left a comment

shubhamvishu commented Oct 12, 2023

benwtrent commented Oct 12, 2023

msokolov left a comment

msokolov Oct 12, 2023

msokolov Oct 12, 2023

shubhamvishu Oct 12, 2023 •

edited

Loading

msokolov Oct 12, 2023

shubhamvishu Oct 12, 2023

msokolov commented Oct 12, 2023

shubhamvishu commented Oct 12, 2023

benwtrent commented Oct 12, 2023

shubhamvishu commented Oct 13, 2023

Ability to compute vector similarity scores with DoubleValuesSource #12548

Ability to compute vector similarity scores with DoubleValuesSource #12548

Conversation

shubhamvishu commented Sep 11, 2023 • edited Loading

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stefanvodita left a comment

Choose a reason for hiding this comment

benwtrent left a comment

Choose a reason for hiding this comment

shubhamvishu commented Oct 12, 2023

benwtrent commented Oct 12, 2023

msokolov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shubhamvishu Oct 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msokolov commented Oct 12, 2023

shubhamvishu commented Oct 12, 2023

benwtrent commented Oct 12, 2023

shubhamvishu commented Oct 13, 2023

shubhamvishu commented Sep 11, 2023 •

edited

Loading

shubhamvishu Oct 12, 2023 •

edited

Loading