Fixed code review comments

Signed-off-by: Vijayan Balasubramanian <[email protected]>

src/main/java/org/opensearch/knn/index/engine/AbstractKNNMethod.java

@@ -122,7 +122,7 @@ public KNNLibraryIndexingContext getKNNLibraryIndexingContext(
             knnMethodConfigContext
         );
         Map<String, Object> parameterMap = knnLibraryIndexingContext.getLibraryParameters();
-        parameterMap.put(KNNConstants.SPACE_TYPE, getCompatibleSpaceType(knnMethodContext.getSpaceType()).getValue());
+        parameterMap.put(KNNConstants.SPACE_TYPE, convertUserToMethodSpaceType(knnMethodContext.getSpaceType()).getValue());
         parameterMap.put(KNNConstants.VECTOR_DATA_TYPE_FIELD, knnMethodConfigContext.getVectorDataType().getValue());
         return KNNLibraryIndexingContextImpl.builder()
             .quantizationConfig(knnLibraryIndexingContext.getQuantizationConfig())
@@ -140,17 +140,19 @@ public KNNLibrarySearchContext getKNNLibrarySearchContext() {
     }
 
     /**
-     * Gets the compatible space type for the given space type parameter.
+     * Converts user defined space type to method space type that is supported by library.
      * The subclass can override this method and returns the appropriate space type that
-     * is compatible with the library.
+     * is supported by the library. This is required because, some libraries may not
+     * support all the space types supported by OpenSearch, however. this can be achieved by using compatible space type by the library.
+     * For example, faiss does not support cosine similarity. However, we can use inner product space type for cosine similarity after normalization.
+     * In this case, we can return the inner product space type for cosine similarity.
      *
      * @param spaceType The space type to check for compatibility
      * @return The compatible space type for the given input, returns the same
      *         space type if it's already compatible
      * @see SpaceType
      */
-
-    protected SpaceType getCompatibleSpaceType(SpaceType spaceType) {
+    protected SpaceType convertUserToMethodSpaceType(SpaceType spaceType) {
         return spaceType;
     }
 }

src/main/java/org/opensearch/knn/index/engine/KNNLibraryIndexingContext.java

@@ -49,5 +49,11 @@ public interface KNNLibraryIndexingContext {
      */
     PerDimensionProcessor getPerDimensionProcessor();
 
+    /**
+     * Get the vector transformer that will be used to transform the vector before indexing.
+     * This will be applied at vector level once entire vector is parsed and validated.
+     *
+     * @return VectorTransformer
+     */
     VectorTransformer getVectorTransformer();
 }

src/main/java/org/opensearch/knn/index/engine/faiss/AbstractFaissMethod.java

@@ -140,13 +140,13 @@ static MethodComponentContext getEncoderMethodComponent(MethodComponentContext m
     }
 
     @Override
-    protected SpaceType getCompatibleSpaceType(SpaceType spaceType) {
+    protected SpaceType convertUserToMethodSpaceType(SpaceType spaceType) {
         // While FAISS doesn't directly support cosine similarity, we can leverage the mathematical
         // relationship between cosine similarity and inner product for normalized vectors to add support.
         // When ||a|| = ||b|| = 1, cos(θ) = a · b
         if (spaceType == SpaceType.COSINESIMIL) {
             return SpaceType.INNER_PRODUCT;
         }
-        return super.getCompatibleSpaceType(spaceType);
+        return super.convertUserToMethodSpaceType(spaceType);
     }
 }

src/main/java/org/opensearch/knn/index/mapper/ModelFieldMapper.java

@@ -5,6 +5,7 @@
 
 package org.opensearch.knn.index.mapper;
 
+import lombok.extern.log4j.Log4j2;
 import org.apache.lucene.document.FieldType;
 import org.apache.lucene.index.DocValuesType;
 import org.apache.lucene.index.VectorEncoding;
@@ -33,6 +34,7 @@
 /**
  * Field mapper for model in mapping
  */
+@Log4j2
 public class ModelFieldMapper extends KNNVectorFieldMapper {
 
     // If the dimension has not yet been set because we do not have access to model metadata, it will be -1
@@ -215,6 +217,9 @@ private void initVectorTransformer() {
         KNNMethodConfigContext knnMethodConfigContext = getKNNMethodConfigContextFromModelMetadata(modelMetadata);
         // Need to handle BWC case
         if (knnMethodContext == null || knnMethodConfigContext == null) {
+            log.debug(
+                "Method Context not available - falling back to Model Metadata for Engine and Space type to determine VectorTransformer instance"
+            );
             vectorTransformer = VectorTransformerFactory.getVectorTransformer(modelMetadata.getKnnEngine(), modelMetadata.getSpaceType());
             return;
         }

src/main/java/org/opensearch/knn/index/mapper/VectorTransformer.java

@@ -5,41 +5,59 @@
 
 package org.opensearch.knn.index.mapper;
 
+import java.util.Arrays;
+
 /**
  * Defines operations for transforming vectors in the k-NN search context.
  * Implementations can modify vectors while preserving their dimensional properties
  * for specific use cases such as normalization, scaling, or other transformations.
- *
- * <p>This interface provides default implementations that pass through the original
- * vector without modification. Implementing classes should override these methods
- * to provide specific transformation logic.
  */
 public interface VectorTransformer {
 
     /**
      * Transforms a float vector into a new vector of the same type.
-     * The default implementation returns the input vector unchanged.
      *
-     * @param vector The input vector to transform
+     * Example:
+     * <pre>{@code
+     * float[] input = {1.0f, 2.0f, 3.0f};
+     * float[] transformed = transformer.transform(input);
+     * }</pre>
+     *
+     * @param vector The input vector to transform (must not be null)
      * @return The transformed vector
+     * @throws IllegalArgumentException if the input vector is null
      */
-    default float[] transform(float[] vector) {
-        return vector;
+    default float[] transform(final float[] vector) {
+        if (vector == null) {
+            throw new IllegalArgumentException("Input vector cannot be null");
+        }
+        return Arrays.copyOf(vector, vector.length);
     }
 
     /**
      * Transforms a byte vector into a new vector of the same type.
-     * The default implementation returns the input vector unchanged.
      *
-     * @param vector The input vector to transform
+     * Example:
+     * <pre>{@code
+     * byte[] input = {1, 2, 3};
+     * byte[] transformed = transformer.transform(input);
+     * }</pre>
+     *
+     * @param vector The input vector to transform (must not be null)
      * @return The transformed vector
+     * @throws IllegalArgumentException if the input vector is null
      */
-    default byte[] transform(byte[] vector) {
-        return vector;
+    default byte[] transform(final byte[] vector) {
+        if (vector == null) {
+            throw new IllegalArgumentException("Input vector cannot be null");
+        }
+        // return copy of vector to avoid side effects
+        return Arrays.copyOf(vector, vector.length);
+
     }
 
     /**
-     * A no-operation transformer that returns vectors unchanged.
+     * A no-operation transformer that returns vector values unchanged.
      * This constant can be used when no transformation is needed.
      */
     VectorTransformer NOOP_VECTOR_TRANSFORMER = new VectorTransformer() {