diff --git a/lucene/CHANGES.txt b/lucene/CHANGES.txt index e38cbe1a216f..5eb9b661fc5b 100644 --- a/lucene/CHANGES.txt +++ b/lucene/CHANGES.txt @@ -226,6 +226,8 @@ Bug Fixes * Make VectorScorer#bulk default method lazily position its iterator on first use. (Michael Marshall) +* GITHUB#16409: Fix filtered HNSW search boundary handling to match the general HNSW searcher. (Mingi Jeong) + Changes in Runtime Behavior --------------------- * GITHUB#14187: The query cache is now disabled by default. (Adrien Grand) diff --git a/lucene/core/src/java/org/apache/lucene/util/hnsw/FilteredHnswGraphSearcher.java b/lucene/core/src/java/org/apache/lucene/util/hnsw/FilteredHnswGraphSearcher.java index a6a281455542..277c8d3e6b73 100644 --- a/lucene/core/src/java/org/apache/lucene/util/hnsw/FilteredHnswGraphSearcher.java +++ b/lucene/core/src/java/org/apache/lucene/util/hnsw/FilteredHnswGraphSearcher.java @@ -131,11 +131,17 @@ void searchLevel( // A bound that holds the minimum similarity to the query vector that a candidate vector must // have to be considered. float minAcceptedSimilarity = Math.nextUp(results.minCompetitiveSimilarity()); + // Allow one candidate equivalent to the current minimum, matching HnswGraphSearcher. + boolean shouldExploreMinSim = true; while (candidates.size() > 0 && results.earlyTerminated() == false) { // get the best candidate (closest or best scoring) float topCandidateSimilarity = candidates.topScore(); - if (minAcceptedSimilarity > topCandidateSimilarity) { - break; + if (topCandidateSimilarity < minAcceptedSimilarity) { + if (shouldExploreMinSim && Math.nextUp(topCandidateSimilarity) == minAcceptedSimilarity) { + shouldExploreMinSim = false; + } else { + break; + } } int topCandidateNode = candidates.pop(); graph.seek(level, topCandidateNode); @@ -197,15 +203,19 @@ void searchLevel( ? scorer.bulkScore(toScore.nodes, bulkScores, toScore.size) : Float.NEGATIVE_INFINITY; results.incVisitedCount(toScore.count()); - if (maxScore > minAcceptedSimilarity) { + if (maxScore > results.minCompetitiveSimilarity()) { for (int i = 0; i < toScore.count(); i++) { int idx = i + toScore.upto; float friendSimilarity = bulkScores[idx]; - if (friendSimilarity > minAcceptedSimilarity) { + if (friendSimilarity >= minAcceptedSimilarity) { int ord = toScore.nodes[idx]; candidates.add(ord, friendSimilarity); if (results.collect(ord, friendSimilarity)) { + float oldMinAcceptedSimilarity = minAcceptedSimilarity; minAcceptedSimilarity = Math.nextUp(results.minCompetitiveSimilarity()); + if (minAcceptedSimilarity > oldMinAcceptedSimilarity) { + shouldExploreMinSim = true; + } } } } diff --git a/lucene/core/src/test/org/apache/lucene/util/hnsw/TestFilteredHnswGraphSearcher.java b/lucene/core/src/test/org/apache/lucene/util/hnsw/TestFilteredHnswGraphSearcher.java new file mode 100644 index 000000000000..d7039fab4020 --- /dev/null +++ b/lucene/core/src/test/org/apache/lucene/util/hnsw/TestFilteredHnswGraphSearcher.java @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.lucene.util.hnsw; + +import java.io.IOException; +import org.apache.lucene.search.KnnCollector; +import org.apache.lucene.search.TopDocs; +import org.apache.lucene.search.TopKnnCollector; +import org.apache.lucene.search.knn.KnnSearchStrategy; +import org.apache.lucene.tests.util.LuceneTestCase; +import org.apache.lucene.util.FixedBitSet; + +public class TestFilteredHnswGraphSearcher extends LuceneTestCase { + + public void testExploresStrictlyCompetitiveBoundaryCandidate() throws IOException { + OnHeapHnswGraph graph = new OnHeapHnswGraph(2, 4); + for (int node = 0; node < 4; node++) { + graph.addNode(0, node); + } + assertTrue(graph.trySetNewEntryNode(0, 0)); + graph.getNeighbors(0, 0).addInOrder(1, 1f); + + float entryScore = 0.5f; + float betterScore = Math.nextUp(entryScore); + float[] scores = {entryScore, betterScore, 0.1f, 0.1f}; + RandomVectorScorer scorer = + new RandomVectorScorer() { + @Override + public float score(int node) { + return scores[node]; + } + + @Override + public int maxOrd() { + return scores.length; + } + }; + + FixedBitSet acceptOrds = new FixedBitSet(4); + acceptOrds.set(0); + acceptOrds.set(1); + KnnCollector collector = + new TopKnnCollector(1, Integer.MAX_VALUE, KnnSearchStrategy.Hnsw.DEFAULT); + + FilteredHnswGraphSearcher.create(collector.k(), graph, 2, acceptOrds) + .search(collector, scorer, graph, acceptOrds); + + TopDocs results = collector.topDocs(); + assertEquals(1, results.scoreDocs.length); + assertEquals(1, results.scoreDocs[0].doc); + assertEquals(betterScore, results.scoreDocs[0].score, 0f); + } + + public void testAllTiedScoresBoundedVisitation() throws IOException { + OnHeapHnswGraph graph = new OnHeapHnswGraph(2, 6); + for (int node = 0; node < 6; node++) { + graph.addNode(0, node); + } + assertTrue(graph.trySetNewEntryNode(0, 0)); + for (int node = 0; node < 5; node++) { + graph.getNeighbors(0, node).addInOrder(node + 1, 1f); + } + + RandomVectorScorer scorer = + new RandomVectorScorer() { + @Override + public float score(int node) { + return 0.5f; + } + + @Override + public int maxOrd() { + return 6; + } + }; + + FixedBitSet acceptOrds = new FixedBitSet(6); + for (int node = 0; node < 4; node++) { + acceptOrds.set(node); + } + KnnCollector collector = + new TopKnnCollector(1, Integer.MAX_VALUE, KnnSearchStrategy.Hnsw.DEFAULT); + + FilteredHnswGraphSearcher.create(collector.k(), graph, 4, acceptOrds) + .search(collector, scorer, graph, acceptOrds); + + // One candidate tied with the current minimum may be explored, then the search must stop + // instead of walking the rest of the equal-scored chain. + assertEquals(2, collector.visitedCount()); + TopDocs results = collector.topDocs(); + assertEquals(1, results.scoreDocs.length); + assertEquals(0, results.scoreDocs[0].doc); + assertEquals(0.5f, results.scoreDocs[0].score, 0f); + } +}