You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2021/10/07 23:46:06 UTC
[GitHub] [lucene] jtibshirani opened a new pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
jtibshirani opened a new pull request #366:
URL: https://github.com/apache/lucene/pull/366
This PR adds support for using cosine similarity with kNN vector fields.
It takes a simple approach and doesn't attempt optimizations like normalizing
the query vector in advance, or performing loop unrolling. The thinking is that
users who prioritize efficiency can normalize all vectors in advance and use
`VectorSimilarityFunction.DOT_PRODUCT`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani merged pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani merged pull request #366:
URL: https://github.com/apache/lucene/pull/366
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani commented on pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #366:
URL: https://github.com/apache/lucene/pull/366#issuecomment-940146857
Thanks for the review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani commented on pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #366:
URL: https://github.com/apache/lucene/pull/366#issuecomment-941239181
@msokolov this is a good point, I will play around with this more and loop back here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mayya-sharipova commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726206695
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
Good notice!
As we store the enum `VectorSimilarityFunction`'s ordinal in `Lucene90HnswVectorsWriter` on disk, declaration order in the enum `VectorSimilarityFunction` is very important, and we should not allow any modifications that change this order.
May be we can write an extra test for `VectorSimilarityFunction` that test for each ordinal we return an expected similarity, e.g. ` VectorSimilarityFunction.values()[0]` always returns `EUCLIDEAN` etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mayya-sharipova commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726247466
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
oh sorry, I see that you've added this already, all is good from my side then
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani commented on pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #366:
URL: https://github.com/apache/lucene/pull/366#issuecomment-947053287
@msokolov @mayya-sharipova following up: I ran benchmarks and it's indeed significantly slower (around 20% on some of the datasets we've been using). Here's what I've done:
* Added a note to `VectorSimilarityFunction#COSINE` explaining that `DOT_PRODUCT` is the preferred option when you don't need to preserve the original vectors: https://github.com/apache/lucene/pull/400
* Opened https://issues.apache.org/jira/browse/LUCENE-10191 with ideas to speed up cosine similarity
Happy to hear other feedback/ ideas!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726227198
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
It's indeed important to prevent this. We actually have a test `BaseKnnVectorsFormatTestCase#testSimilarityFunctionIdentifiers` that checks the ordinals.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] jtibshirani commented on pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
jtibshirani commented on pull request #366:
URL: https://github.com/apache/lucene/pull/366#issuecomment-941239181
@msokolov this is a good point, I will play around with this more and loop back here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] msokolov commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r727213376
##########
File path: lucene/core/src/test/org/apache/lucene/search/TestKnnVectorQuery.java
##########
@@ -234,10 +235,65 @@ public void testScoreDotProduct() throws IOException {
// test getMaxScore
assertEquals(0, scorer.getMaxScore(-1), 0);
- /* maxAtZero = ((2,3) * (1, 1) = 5) / (||2, 3|| * ||1, 1|| = sqrt(26)) = 0.5, then
+ /* maxAtZero = ((2,3) * (1, 1) = 5) / (||2, 3|| * ||1, 1|| = sqrt(26)), then
* normalized by (1 + x) /2.
*/
- float maxAtZero = 0.99029f;
+ float maxAtZero =
+ (float) ((1 + (2 * 1 + 3 * 1) / Math.sqrt((2 * 2 + 3 * 3) * (1 * 1 + 1 * 1))) / 2);
Review comment:
thanks for the cleanup here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mayya-sharipova commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726206695
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
Good notice!
As we store the enum `VectorSimilarityFunction`'s ordinal in `Lucene90HnswVectorsWriter` on disk, declaration order in the enum `VectorSimilarityFunction` is very important, and we should not allow any modifications that change this order.
May be we can write an extra test for `VectorSimilarityFunction` that tests for each ordinal we return an expected similarity, e.g. ` VectorSimilarityFunction.values()[0]` always returns `EUCLIDEAN`, ..[1] -> `DOT_PRODUCT` etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mayya-sharipova commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726206695
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
Good notice!
As we store the enum `VectorSimilarityFunction`'s ordinal in `Lucene90HnswVectorsWriter` on disk, declaration order in the enum `VectorSimilarityFunction` is very important, and we should not allow any modifications that change this order.
May be we can write an extra test for `VectorSimilarityFunction` that tests for each ordinal we return an expected similarity, e.g. ` VectorSimilarityFunction.values()[0]` always returns `EUCLIDEAN` etc.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] mayya-sharipova commented on a change in pull request #366: LUCENE-10146: Add VectorSimilarityFunction.COSINE
Posted by GitBox <gi...@apache.org>.
mayya-sharipova commented on a change in pull request #366:
URL: https://github.com/apache/lucene/pull/366#discussion_r726246065
##########
File path: lucene/core/src/java/org/apache/lucene/codecs/lucene90/Lucene90FieldInfosFormat.java
##########
@@ -99,11 +99,12 @@
* <li>PointDimensionCount, PointNumBytes: these are non-zero only if the field is indexed as
* points, e.g. using {@link org.apache.lucene.document.LongPoint}
* <li>VectorDimension: it is non-zero if the field is indexed as vectors.
- * <li>VectorDistFunction: a byte containing distance function used for similarity calculation.
+ * <li>VectorSimilarityFunction: a byte containing distance function used for similarity
+ * calculation.
* <ul>
- * <li>0: no distance function is defined for this field.
- * <li>1: EUCLIDEAN_HNSW distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
- * <li>2: DOT_PRODUCT_HNSW score. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>0: EUCLIDEAN distance. ({@link VectorSimilarityFunction#EUCLIDEAN})
+ * <li>1: DOT_PRODUCT similarity. ({@link VectorSimilarityFunction#DOT_PRODUCT})
+ * <li>2: COSINE similarity. ({@link VectorSimilarityFunction#COSINE})
Review comment:
Thanks for letting me know about this. may be we can add a new COSINE similarity there as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org