jtibshirani commented on a change in pull request #277:
URL: https://github.com/apache/lucene/pull/277#discussion_r702202529
##########
File path: lucene/core/src/test/org/apache/lucene/util/hnsw/TestHnswGraph.java
##########
@@ -493,9 +496,14 @@ public RandomVectorValues copy() {
}
/** Generate a random bitset where each entry has a 2/3 probability of being
set. */
- private static Bits createRandomAcceptOrds(int length) {
+ private static Bits createRandomAcceptOrds(int startIndex, int length) {
Review comment:
I think the method comment could use an update, now that we have
`startIndex`.
##########
File path: lucene/core/src/test/org/apache/lucene/util/hnsw/TestHnswGraph.java
##########
@@ -167,24 +167,27 @@ public void testSearchWithAcceptOrds() throws IOException
{
vectors, VectorSimilarityFunction.DOT_PRODUCT, 16, 100,
random().nextInt());
HnswGraph hnsw = builder.build(vectors);
- Bits acceptOrds = createRandomAcceptOrds(vectors.size);
+ // the first 10 docs must not be deleted to ensure the expected recall
+ Bits acceptOrds = createRandomAcceptOrds(10, vectors.size);
NeighborQueue nn =
HnswGraph.search(
new float[] {1, 0},
10,
- 5,
+ 10,
vectors.randomAccess(),
VectorSimilarityFunction.DOT_PRODUCT,
hnsw,
acceptOrds,
random());
int sum = 0;
- for (int node : nn.nodes()) {
+ int[] nodes = nn.nodes();
+ assert (nodes.length == 10);
Review comment:
Maybe this could be a test assertion instead of an `assert`.
##########
File path: lucene/core/src/test/org/apache/lucene/util/hnsw/TestHnswGraph.java
##########
@@ -167,24 +167,27 @@ public void testSearchWithAcceptOrds() throws IOException
{
vectors, VectorSimilarityFunction.DOT_PRODUCT, 16, 100,
random().nextInt());
HnswGraph hnsw = builder.build(vectors);
- Bits acceptOrds = createRandomAcceptOrds(vectors.size);
+ // the first 10 docs must not be deleted to ensure the expected recall
+ Bits acceptOrds = createRandomAcceptOrds(10, vectors.size);
NeighborQueue nn =
HnswGraph.search(
new float[] {1, 0},
10,
- 5,
+ 10,
Review comment:
It'd be nice to fix the other places in this test that use 5 instead of
10. From the comments like "the lowest docIds are closest to zero; sum(0,9) =
45", I'm guessing it was an accident too.
--
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-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
