Skip to content

Commit

Permalink
Update legacy mapping
Browse files Browse the repository at this point in the history
Signed-off-by: Vijayan Balasubramanian <[email protected]>
  • Loading branch information
VijayanB committed Oct 20, 2024
1 parent 344f694 commit daf89f6
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ static KNNMethodContext createKNNMethodContextFromLegacy(
? topLevelSpaceType
: KNNVectorFieldMapperUtil.getSpaceType(indexSettings);
return new KNNMethodContext(
KNNEngine.NMSLIB,
KNNEngine.DEFAULT,
finalSpaceToSet,
new MethodComponentContext(
METHOD_HNSW,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

public class KNNIndexShardTests extends KNNSingleNodeTestCase {

public static final String FAISS_HNSW_EXTENSION = "faiss";
private final String testIndexName = "test-index";
private final String testFieldName = "test-field";
private final int dimensions = 2;
Expand Down Expand Up @@ -114,7 +115,7 @@ public void testGetAllEngineFileContexts() throws IOException, ExecutionExceptio
engineFileContexts = knnIndexShard.getAllEngineFileContexts(searcher.getIndexReader());
assertEquals(1, engineFileContexts.size());
List<String> paths = engineFileContexts.stream().map(KNNIndexShard.EngineFileContext::getIndexPath).collect(Collectors.toList());
assertTrue(paths.get(0).contains("hnsw") || paths.get(0).contains("hnswc"));
assertTrue(paths.get(0).contains(FAISS_HNSW_EXTENSION));
searcher.close();
}

Expand Down
2 changes: 2 additions & 0 deletions src/test/java/org/opensearch/knn/index/NmslibIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ public void testCreateIndexWithValidAlgoParams_mappingAndSettings() {
.field("dimension", 2)
.startObject(KNNConstants.KNN_METHOD)
.field(KNNConstants.METHOD_PARAMETER_SPACE_TYPE, spaceType1)
.field(KNN_ENGINE, KNNEngine.NMSLIB.getName())
.field(KNNConstants.NAME, KNNConstants.METHOD_HNSW)
.startObject(KNNConstants.PARAMETERS)
.field(KNNConstants.METHOD_PARAMETER_EF_CONSTRUCTION, efConstruction1)
Expand All @@ -378,6 +379,7 @@ public void testCreateIndexWithValidAlgoParams_mappingAndSettings() {
.field("dimension", 2)
.startObject(KNNConstants.KNN_METHOD)
.field(KNNConstants.METHOD_PARAMETER_SPACE_TYPE, spaceType2)
.field(KNN_ENGINE, KNNEngine.NMSLIB.getName())
.field(KNNConstants.NAME, KNNConstants.METHOD_HNSW)
.startObject(KNNConstants.PARAMETERS)
.field(KNNConstants.METHOD_PARAMETER_EF_CONSTRUCTION, efConstruction2)
Expand Down
23 changes: 0 additions & 23 deletions src/test/java/org/opensearch/knn/index/VectorDataTypeIT.java
Original file line number Diff line number Diff line change
Expand Up @@ -263,29 +263,6 @@ public void testByteVectorDataTypeWithNmslibEngine() {
assertTrue(ex.getMessage().contains("is not supported for vector data type"));
}

@SneakyThrows
public void testByteVectorDataTypeWithLegacyFieldMapperKnnIndexSetting() {
// Create an index with byte vector data_type and index.knn as true without setting KnnMethodContext,
// which should throw an exception because the LegacyFieldMapper will use NMSLIB engine and byte data_type
// is not supported for NMSLIB engine.
XContentBuilder builder = XContentFactory.jsonBuilder()
.startObject()
.startObject(PROPERTIES_FIELD)
.startObject(FIELD_NAME)
.field(TYPE_FIELD_NAME, KNN_VECTOR_TYPE)
.field(DIMENSION, 2)
.field(VECTOR_DATA_TYPE_FIELD, VectorDataType.BYTE.getValue())
.endObject()
.endObject()
.endObject();

String mapping = builder.toString();

ResponseException ex = expectThrows(ResponseException.class, () -> createKnnIndex(INDEX_NAME, mapping));
assertTrue(ex.getMessage(), ex.getMessage().contains("is not supported for vector data type"));

}

public void testDocValuesWithByteVectorDataTypeLuceneEngine() throws Exception {
createKnnIndexMappingWithLuceneEngine(2, SpaceType.L2, VectorDataType.BYTE.getValue());
ingestL2ByteTestData();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,7 @@ public void testAddKNNBinaryField_fromScratch_nmslibLegacy() throws IOException
.addAttribute(KNNConstants.HNSW_ALGO_EF_CONSTRUCTION, "512")
.addAttribute(KNNConstants.HNSW_ALGO_M, "16")
.addAttribute(KNNConstants.SPACE_TYPE, spaceType.getValue())
.addAttribute(KNNConstants.KNN_ENGINE, knnEngine.getName())
.build() };

FieldInfos fieldInfos = new FieldInfos(fieldInfoArray);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@
* Test used for testing Codecs
*/
public class KNNCodecTestCase extends KNNTestCase {
public static final String FAISS_HNSW_EXTENSION = "faiss";
private static final FieldType sampleFieldType;
static {
KNNMethodConfigContext knnMethodConfigContext = KNNMethodConfigContext.builder()
Expand Down Expand Up @@ -187,7 +188,7 @@ public void testMultiFieldsKnnIndex(Codec codec) throws Exception {
writer.close();
ResourceWatcherService resourceWatcherService = createDisabledResourceWatcherService();
NativeMemoryLoadStrategy.IndexLoadStrategy.initialize(resourceWatcherService);
List<String> hnswfiles = Arrays.stream(dir.listAll()).filter(x -> x.contains("hnsw")).collect(Collectors.toList());
List<String> hnswfiles = Arrays.stream(dir.listAll()).filter(x -> x.contains(FAISS_HNSW_EXTENSION)).collect(Collectors.toList());

// there should be 2 hnsw index files created. one for test_vector and one for my_vector
assertEquals(2, hnswfiles.size());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ public void testTypeParser_build_fromKnnMethodContext() throws IOException {
// Check that knnMethodContext takes precedent over both model and legacy
ModelDao modelDao = mock(ModelDao.class);

SpaceType spaceType = SpaceType.COSINESIMIL;
SpaceType spaceType = SpaceType.DEFAULT;
int mRight = 17;
int mWrong = 71;

Expand Down Expand Up @@ -1614,7 +1614,7 @@ public void testBuilder_whenBinaryWithLegacyKNNDisabled_thenValid() {
assertTrue(knnVectorFieldMapper instanceof FlatVectorFieldMapper);
}

public void testTypeParser_whenBinaryWithLegacyKNNEnabled_thenException() throws IOException {
public void testTypeParser_whenBinaryWithLegacyKNNEnabled_thenValid() throws IOException {
// Check legacy is picked up if model context and method context are not set
ModelDao modelDao = mock(ModelDao.class);
KNNVectorFieldMapper.TypeParser typeParser = new KNNVectorFieldMapper.TypeParser(() -> modelDao);
Expand All @@ -1631,11 +1631,12 @@ public void testTypeParser_whenBinaryWithLegacyKNNEnabled_thenException() throws
.field(VECTOR_DATA_TYPE_FIELD, VectorDataType.BINARY.getValue())
.endObject();

Exception ex = expectThrows(Exception.class, () -> {
typeParser.parse(fieldName, xContentBuilderToMap(xContentBuilder), buildParserContext(indexName, settings));
});

assertTrue(ex.getMessage(), ex.getMessage().contains("does not support space type"));
final KNNVectorFieldMapper.Builder builder = (KNNVectorFieldMapper.Builder) typeParser.parse(
fieldName,
xContentBuilderToMap(xContentBuilder),
buildParserContext(indexName, settings)
);
assertEquals(SpaceType.HAMMING, builder.getOriginalParameters().getResolvedKnnMethodContext().getSpaceType());
}

public void testBuild_whenInvalidCharsInFieldName_thenThrowException() {
Expand All @@ -1661,7 +1662,7 @@ public void testTypeParser_whenModeAndCompressionAreSet_thenHandle() throws IOEx
ModelDao modelDao = mock(ModelDao.class);
KNNVectorFieldMapper.TypeParser typeParser = new KNNVectorFieldMapper.TypeParser(() -> modelDao);

// Default to nmslib and ensure legacy is in use
// Default to faiss and ensure legacy is in use
XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()
.startObject()
.field(TYPE_FIELD_NAME, KNN_VECTOR_TYPE)
Expand All @@ -1676,7 +1677,7 @@ public void testTypeParser_whenModeAndCompressionAreSet_thenHandle() throws IOEx
assertTrue(builder.getOriginalParameters().isLegacyMapping());
validateBuilderAfterParsing(
builder,
KNNEngine.NMSLIB,
KNNEngine.DEFAULT,
SpaceType.L2,
VectorDataType.FLOAT,
CompressionLevel.x1,
Expand All @@ -1702,7 +1703,7 @@ public void testTypeParser_whenModeAndCompressionAreSet_thenHandle() throws IOEx
assertFalse(builder.getOriginalParameters().isLegacyMapping());
validateBuilderAfterParsing(
builder,
KNNEngine.NMSLIB,
KNNEngine.DEFAULT,
SpaceType.L2,
VectorDataType.FLOAT,
CompressionLevel.x1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ public class KNNWeightTests extends KNNTestCase {
private static final int K = 5;
private static final Set<String> SEGMENT_FILES_NMSLIB = Set.of("_0.cfe", "_0_2011_target_field.hnswc");
private static final Set<String> SEGMENT_FILES_FAISS = Set.of("_0.cfe", "_0_2011_target_field.faissc");
private static final Set<String> SEGMENT_FILES_DEFAULT = SEGMENT_FILES_FAISS;
private static final Set<String> SEGMENT_MULTI_FIELD_FILES_FAISS = Set.of(
"_0.cfe",
"_0_2011_target_field.faissc",
Expand Down Expand Up @@ -398,7 +399,7 @@ public void testEmptyQueryResults() {
Map.of(),
Sort.RELEVANCE
);
segmentInfo.setFiles(SEGMENT_FILES_NMSLIB);
segmentInfo.setFiles(SEGMENT_FILES_DEFAULT);
final SegmentCommitInfo segmentCommitInfo = new SegmentCommitInfo(segmentInfo, 0, 0, 0, 0, 0, new byte[StringHelper.ID_LENGTH]);
when(reader.getSegmentInfo()).thenReturn(segmentCommitInfo);

Expand Down
Loading

0 comments on commit daf89f6

Please sign in to comment.