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/04/25 19:11:01 UTC

[GitHub] [lucene] msokolov opened a new pull request #106: LUCENE-9905: rename VectorValues.SearchStrategy to VectorValues.SimilarityFunction

msokolov opened a new pull request #106:
URL: https://github.com/apache/lucene/pull/106


   


-- 
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.

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 pull request #106: LUCENE-9905: rename VectorValues.SearchStrategy to VectorValues.SimilarityFunction

Posted by GitBox <gi...@apache.org>.
msokolov commented on pull request #106:
URL: https://github.com/apache/lucene/pull/106#issuecomment-828422200


   I merged this separately


-- 
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.

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 #106: LUCENE-9905: rename VectorValues.SearchStrategy to VectorValues.SimilarityFunction

Posted by GitBox <gi...@apache.org>.
msokolov commented on a change in pull request #106:
URL: https://github.com/apache/lucene/pull/106#discussion_r621665922



##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -2336,6 +2338,29 @@ static void checkImpacts(Impacts impacts, int lastTarget) {
                       + docCount
                       + " docs with values");
             }
+            VectorReader vectorReader = reader.getVectorReader();
+            if (vectorReader instanceof Lucene90HnswVectorReader) {
+              KnnGraphValues graphValues =
+                  ((Lucene90HnswVectorReader) vectorReader).getGraphValues(fieldInfo.name);
+              int size = graphValues.size();
+              for (int i = 0; i < size; i++) {
+                graphValues.seek(i);
+                for (int neighbor = graphValues.nextNeighbor();
+                    neighbor != NO_MORE_DOCS;
+                    neighbor = graphValues.nextNeighbor()) {
+                  if (neighbor < 0 || neighbor >= size) {
+                    throw new RuntimeException(
+                        "Field \""
+                            + fieldInfo.name
+                            + "\" has an invalid neighbor ordinal: "
+                            + neighbor
+                            + " which should be in [0,"
+                            + size
+                            + ")");
+                  }
+                }
+              }
+            }

Review comment:
       Ah, this slipped in here by accident. I'll remove and add back in a separate commit. My understanding about CheckIndex may be incomplete - I thought it was mostly intended as an operational testing and recovery tool, but I think you're saying it's part if the unit test framework?




-- 
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.

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 closed pull request #106: LUCENE-9905: rename VectorValues.SearchStrategy to VectorValues.SimilarityFunction

Posted by GitBox <gi...@apache.org>.
msokolov closed pull request #106:
URL: https://github.com/apache/lucene/pull/106


   


-- 
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.

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] jpountz commented on a change in pull request #106: LUCENE-9905: rename VectorValues.SearchStrategy to VectorValues.SimilarityFunction

Posted by GitBox <gi...@apache.org>.
jpountz commented on a change in pull request #106:
URL: https://github.com/apache/lucene/pull/106#discussion_r620046351



##########
File path: lucene/core/src/java/org/apache/lucene/index/CheckIndex.java
##########
@@ -2336,6 +2338,29 @@ static void checkImpacts(Impacts impacts, int lastTarget) {
                       + docCount
                       + " docs with values");
             }
+            VectorReader vectorReader = reader.getVectorReader();
+            if (vectorReader instanceof Lucene90HnswVectorReader) {
+              KnnGraphValues graphValues =
+                  ((Lucene90HnswVectorReader) vectorReader).getGraphValues(fieldInfo.name);
+              int size = graphValues.size();
+              for (int i = 0; i < size; i++) {
+                graphValues.seek(i);
+                for (int neighbor = graphValues.nextNeighbor();
+                    neighbor != NO_MORE_DOCS;
+                    neighbor = graphValues.nextNeighbor()) {
+                  if (neighbor < 0 || neighbor >= size) {
+                    throw new RuntimeException(
+                        "Field \""
+                            + fieldInfo.name
+                            + "\" has an invalid neighbor ordinal: "
+                            + neighbor
+                            + " which should be in [0,"
+                            + size
+                            + ")");
+                  }
+                }
+              }
+            }

Review comment:
       Codec-specific tests do not feel like a good fit for CheckIndex, should it move to TestLucene90HnswVectorFormat?




-- 
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.

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