Adds support for ef_search query parameter in FAISS engine (opensearc…

…h-project#1707) * Adds support for ef_search as a query parameter in faiss engine Signed-off-by: Tejas Shah <[email protected]> * Fixing checkstyle Signed-off-by: Tejas Shah <[email protected]> * Corrects imports Signed-off-by: Tejas Shah <[email protected]> * Adds unit test for faiss_wrapper for queryIndex and queryIndexWithFilter methods This unit test specifically focuses on the changes related to ef_search Detailed assertions related to grouper and selector can be picked up later. Tests related to any other methods are not added Signed-off-by: Tejas Shah <[email protected]> * Gates efSearch behind 2.15 version to make sure there is backward compatibility during upgrades Signed-off-by: Tejas Shah <[email protected]> * Adds faiss_wrapper_unit_test.cpp in CmakeList Signed-off-by: Tejas Shah <[email protected]> * Adds change log Signed-off-by: Tejas Shah <[email protected]> * Adds BWC tests for efSearch Signed-off-by: Tejas Shah <[email protected]> * Removes EfSearch validation from rolling upgrade test for Mixed cluster state Customers are not expected use the query with new parameters unless its fully upgraded. Signed-off-by: Tejas Shah <[email protected]> * Changing minimum version required for efSearch to be 3.0.0 This makes sure that bwc passes in the PR. will be changed to 2.15 to once this is merged to 2.x Signed-off-by: Tejas Shah <[email protected]> * Fixes the checkstyle failure Signed-off-by: Tejas Shah <[email protected]> * Adds method_parameter in knn search request. method_parameters will be used to hold shard level algorithm parameters Signed-off-by: Tejas Shah <[email protected]> * Updates the documentation for faiss wrapper Signed-off-by: Tejas Shah <[email protected]> * Stubs out method parameter parsing logic into its own class Signed-off-by: Tejas Shah <[email protected]> * Adds method parameters and validates against engine specific parameters Signed-off-by: Tejas Shah <[email protected]> * Removes engine and method functions from EngineSpecificMethodContext Signed-off-by: Tejas Shah <[email protected]> * - Does not throw in case of illegal state exception if method is null - Changes HnswContext to DefaultHnswContext - corrects * imports Signed-off-by: Tejas Shah <[email protected]> --------- Signed-off-by: Tejas Shah <[email protected]>
shatejas · Jun 27, 2024 · a27f787 · a27f787
1 parent 98ea1d0
commit a27f787
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 39 deletions.
diff --git a/jni/src/faiss_wrapper.cpp b/jni/src/faiss_wrapper.cpp
@@ -356,7 +356,6 @@ jobjectArray knn_jni::faiss_wrapper::QueryIndex_WithFilter(knn_jni::JNIUtilInter
         } else {
             auto ivfReader = dynamic_cast<const faiss::IndexIVF*>(indexReader->index);
             auto ivfFlatReader = dynamic_cast<const faiss::IndexIVFFlat*>(indexReader->index);
-
             if(ivfReader || ivfFlatReader) {
                 int indexNprobe = ivfReader == nullptr ? ivfReader->nprobe : ivfFlatReader->nprobe;
                 ivfParams.nprobe = commons::getIntegerMethodParameter(env, jniUtil, methodParams, NPROBES, indexNprobe);

diff --git a/jni/tests/faiss_wrapper_test.cpp b/jni/tests/faiss_wrapper_test.cpp
@@ -287,6 +287,7 @@ TEST(FaissQueryIndexTest, BasicAssertions) {
             delete it;
         }
     }
+    std::cout << "Test end";
 }
 
 //Test for a bug reported in https://github.com/opensearch-project/k-NN/issues/1435

diff --git a/src/main/java/org/opensearch/knn/index/MethodComponent.java b/src/main/java/org/opensearch/knn/index/MethodComponent.java
@@ -117,43 +117,6 @@ public ValidationException validateWithData(MethodComponentContext methodCompone
         return validationException;
     }
 
-    /**
-     * Validate that the methodComponentContext is a valid configuration for this methodComponent, using additional data not present in the method component context
-     *
-     * @param methodComponentContext to be validated
-     * @param vectorSpaceInfo additional data not present in the method component context
-     * @return ValidationException produced by validation errors; null if no validations errors.
-     */
-    public ValidationException validateWithData(MethodComponentContext methodComponentContext, VectorSpaceInfo vectorSpaceInfo) {
-        Map<String, Object> providedParameters = methodComponentContext.getParameters();
-        List<String> errorMessages = new ArrayList<>();
-
-        if (providedParameters == null) {
-            return null;
-        }
-
-        ValidationException parameterValidation;
-        for (Map.Entry<String, Object> parameter : providedParameters.entrySet()) {
-            if (!parameters.containsKey(parameter.getKey())) {
-                errorMessages.add(String.format("Invalid parameter for method \"%s\".", getName()));
-                continue;
-            }
-
-            parameterValidation = parameters.get(parameter.getKey()).validateWithData(parameter.getValue(), vectorSpaceInfo);
-            if (parameterValidation != null) {
-                errorMessages.addAll(parameterValidation.validationErrors());
-            }
-        }
-
-        if (errorMessages.isEmpty()) {
-            return null;
-        }
-
-        ValidationException validationException = new ValidationException();
-        validationException.addValidationErrors(errorMessages);
-        return validationException;
-    }
-
     /**
      * gets requiresTraining value
      *

diff --git a/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java b/src/main/java/org/opensearch/knn/index/query/KNNQueryBuilder.java
@@ -217,13 +217,16 @@ private void validate() {
                 throw new IllegalArgumentException(String.format("[%s] requires exactly one of k, distance or score to be set", NAME));
             }
 
+            VectorQueryType vectorQueryType = VectorQueryType.MAX_DISTANCE;
             if (k != null) {
+                vectorQueryType = VectorQueryType.K;
                 if (k <= 0 || k > K_MAX) {
                     throw new IllegalArgumentException(String.format("[%s] requires k to be in the range (0, %d]", NAME, K_MAX));
                 }
             }
 
             if (minScore != null) {
+                vectorQueryType = VectorQueryType.MIN_SCORE;
                 if (minScore <= 0) {
                     throw new IllegalArgumentException(String.format("[%s] requires minScore to be greater than 0", NAME));
                 }
@@ -237,6 +240,12 @@ private void validate() {
                     );
                 }
             }
+
+            // Update stats
+            vectorQueryType.getQueryStatCounter().increment();
+            if (filter != null) {
+                vectorQueryType.getQueryWithFilterStatCounter().increment();
+            }
         }
     }
 

diff --git a/src/test/java/org/opensearch/knn/index/util/AbstractKNNLibraryTests.java b/src/test/java/org/opensearch/knn/index/util/AbstractKNNLibraryTests.java
@@ -11,7 +11,13 @@
 import org.opensearch.common.xcontent.XContentFactory;
 import org.opensearch.knn.KNNTestCase;
 import org.opensearch.knn.common.KNNConstants;
-import org.opensearch.knn.index.*;
+import org.opensearch.knn.index.KNNMethod;
+import org.opensearch.knn.index.KNNMethodContext;
+import org.opensearch.knn.index.MethodComponent;
+import org.opensearch.knn.index.MethodComponentContext;
+import org.opensearch.knn.index.Parameter;
+import org.opensearch.knn.index.SpaceType;
+import org.opensearch.knn.index.VectorQueryType;
 
 import java.io.IOException;
 import java.util.Collections;