Use the Lucene Distance Calculation Function in Script Scoring for doing exact search #1699
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ | |
| import java.util.Objects; | ||
| import org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
| import org.apache.lucene.util.VectorUtil; | ||
| import org.opensearch.knn.index.KNNVectorScriptDocValues; | ||
| import org.opensearch.knn.index.SpaceType; | ||
| import org.opensearch.knn.index.VectorDataType; | ||
|
|
@@ -48,13 +49,7 @@ private static void requireEqualDimension(final float[] queryVector, final float | |
| * @return L2 score | ||
| */ | ||
| public static float l2Squared(float[] queryVector, float[] inputVector) { | ||
| return VectorUtil.squareDistance(queryVector, inputVector); | ||
| } | ||
|
|
||
| private static float[] toFloat(List<Number> inputVector, VectorDataType vectorDataType) { | ||
|
|
@@ -101,19 +96,16 @@ public static float l2Squared(List<Number> queryVector, KNNVectorScriptDocValues | |
| * @return cosine score | ||
| */ | ||
| public static float cosinesimilOptimized(float[] queryVector, float[] inputVector, float normQueryVector) { | ||
| float normInputVector = 0.0f; | ||
| for (int i = 0; i < queryVector.length; i++) { | ||
| normInputVector += inputVector[i] * inputVector[i]; | ||
| } | ||
| float normalizedProduct = normQueryVector * normInputVector; | ||
| if (normalizedProduct == 0) { | ||
| logger.debug("Invalid vectors for cosine. Returning minimum score to put this result to end"); | ||
| return 0.0f; | ||
| } | ||
| return (float) (VectorUtil.dotProduct(queryVector, inputVector) / (Math.sqrt(normalizedProduct))); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -148,20 +140,28 @@ public static float cosineSimilarity(List<Number> queryVector, KNNVectorScriptDo | |
| */ | ||
| public static float cosinesimil(float[] queryVector, float[] inputVector) { | ||
| requireEqualDimension(queryVector, inputVector); | ||
| int numZeroInInput = 0; | ||
|
There was a problem hiding this comment. I think these are unnecessary: https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/util/VectorUtil.java#L79. Can we just do this? There was a problem hiding this comment. That method would still return true for a zero vector right? There was a problem hiding this comment. I believe cosine will be infinite if one vector is finite There was a problem hiding this comment. As long as we validate that it's not zero vector in the above method, we should be able to remove the other check because of the assert finite |
||
| int numZeroInQuery = 0; | ||
| float cosine = 0.0f; | ||
| for (int i = 0; i < inputVector.length; i++) { | ||
| if (inputVector[i] == 0) { | ||
| numZeroInInput++; | ||
| } | ||
|
|
||
| if (queryVector[i] == 0) { | ||
| numZeroInQuery++; | ||
| } | ||
| } | ||
| if (numZeroInInput == inputVector.length || numZeroInQuery == queryVector.length) { | ||
| return cosine; | ||
| } | ||
| try { | ||
| cosine = VectorUtil.cosine(queryVector, inputVector); | ||
| } catch (IllegalArgumentException e) { | ||
|
There was a problem hiding this comment. did lucene doesn't have cosine functions directly present which we can leverage? There was a problem hiding this comment. We use the lucene cosine function on line 159. The rest just returns 0 if either the input or query vectors are all 0's. |
||
| logger.debug("Invalid vectors for cosine. Returning minimum score to put this result to end"); | ||
| return 0.0f; | ||
| } | ||
| return cosine; | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -217,7 +217,6 @@ public static float calculateHammingBit(Long queryLong, Long inputLong) { | |
| * @return L1 score | ||
| */ | ||
| public static float l1Norm(float[] queryVector, float[] inputVector) { | ||
| float distance = 0; | ||
| for (int i = 0; i < inputVector.length; i++) { | ||
| float diff = queryVector[i] - inputVector[i]; | ||
|
|
@@ -255,7 +254,6 @@ public static float l1Norm(List<Number> queryVector, KNNVectorScriptDocValues do | |
| * @return L-inf score | ||
| */ | ||
| public static float lInfNorm(float[] queryVector, float[] inputVector) { | ||
| float distance = 0; | ||
| for (int i = 0; i < inputVector.length; i++) { | ||
| float diff = queryVector[i] - inputVector[i]; | ||
|
|
@@ -293,12 +291,7 @@ public static float lInfNorm(List<Number> queryVector, KNNVectorScriptDocValues | |
| * @return dot product score | ||
| */ | ||
| public static float innerProduct(float[] queryVector, float[] inputVector) { | ||
| return VectorUtil.dotProduct(queryVector, inputVector); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
if we want to use dotProduct / normalize for calculate cosine. this would do one more iteration as original L108 doing dotProduct.
PS, i checked
Lucene#DefaultVectorUtilSupport#cosine(float, float)would do cosine normalizeThere 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.
I used that below, I'll see if I can get it to work with the normVector present in this method
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.
i see there is
return (float) (sum / Math.sqrt((double) norm1 * (double) norm2));inLucene#DefaultVectorUtilSupport#cosine(float, float)so we can use it directly inpublic static float cosinesimilOptimizedand without using dotProductThere 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.
Where are you seeing the
DefaultVectorUtilSupportclass? I've only been able to findVectorUtilso far and that class doesn't have a cosine method that takes floats, only float[]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.
https://github.com/apache/lucene/blob/main/lucene/core/src/java/org/apache/lucene/internal/vectorization/DefaultVectorUtilSupport.java#L65
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.
Looks like it's also used in KNNScoringSpace as well: https://github.com/opensearch-project/k-NN/blob/main/src/main/java/org/opensearch/knn/plugin/script/KNNScoringSpace.java#L106
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.
SIMD would be better as per out older experiments of SIMD. Also, given that lucene lacks that implementation I fine to remove this optimize cosine code for now.
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.
I can see multiple versions of
cosinesimilandcosineSimilarity. Lets just move towards 1 where we use Lucene functions to do the distance calculations and remove all others.Some are using optimized and some doesn't. Lets just clean things up and move towards 1 implementation.
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.
Sure, I'll incorporate that with this PR then
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.
IMHO for those that are very serious about performance, they will normalize their data during preprocessing and use inner product directly. So, I think its okay to not change cosine functionality for now and just focus on dot product and l2 for this optimization.