From 90e406a9b9dfec4e56a21f537fa6319712320886 Mon Sep 17 00:00:00 2001 From: ShuaiHua Du Date: Wed, 3 Jul 2024 00:13:55 +0800 Subject: [PATCH 1/3] Fix milvus search metric type bug. --- .../Connectors/Connectors.Memory.Milvus/MilvusMemoryStore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dotnet/src/Connectors/Connectors.Memory.Milvus/MilvusMemoryStore.cs b/dotnet/src/Connectors/Connectors.Memory.Milvus/MilvusMemoryStore.cs index 38d10778a723..7bdd2f03db94 100644 --- a/dotnet/src/Connectors/Connectors.Memory.Milvus/MilvusMemoryStore.cs +++ b/dotnet/src/Connectors/Connectors.Memory.Milvus/MilvusMemoryStore.cs @@ -446,7 +446,7 @@ public Task RemoveBatchAsync(string collectionName, IEnumerable keys, Ca MilvusCollection collection = this.Client.GetCollection(collectionName); SearchResults results = await collection - .SearchAsync(EmbeddingFieldName, [embedding], SimilarityMetricType.Ip, limit, this._searchParameters, cancellationToken) + .SearchAsync(EmbeddingFieldName, [embedding], this._metricType, limit, this._searchParameters, cancellationToken) .ConfigureAwait(false); IReadOnlyList ids = results.Ids.StringIds!; From ea9d37208dcf47e4d34b700bb74fe59f0bda3492 Mon Sep 17 00:00:00 2001 From: ShuaiHua Du Date: Wed, 3 Jul 2024 02:37:00 +0800 Subject: [PATCH 2/3] Add milvus connector unit tests, perform the search using different metrics. --- .../Memory/Milvus/MilvusMemoryStoreTests.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs b/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs index 0ed028eba747..64edf1280f1b 100644 --- a/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs +++ b/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs @@ -220,6 +220,47 @@ public async Task GetNearestMatchesAsync(bool withEmbeddings) }); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public async Task GetNearestMatchesWithMetricTypeAsync(bool withEmbeddings) + { + //Create collection with default, Ip metric + await this.Store.CreateCollectionAsync(CollectionName); + await this.InsertSampleDataAsync(); + // There seems to be some race condition where the upserted data above isn't taken into account in the search below and zero results are returned... + await Task.Delay(1000); + + //Search with Ip metric, run correctly + List<(MemoryRecord Record, double SimilarityScore)> ipResults = + this.Store.GetNearestMatchesAsync(CollectionName, new[] { 5f, 6f, 7f, 8f, 9f }, limit: 2, withEmbeddings: withEmbeddings).ToEnumerable().ToList(); + + Assert.All(ipResults, t => Assert.True(t.SimilarityScore > 0)); + + //Set the store to Cosine metric, without recreate collection + this.Store = new(this._milvusFixture.Host, vectorSize: 5, port: this._milvusFixture.Port, metricType: SimilarityMetricType.Cosine, consistencyLevel: ConsistencyLevel.Strong); + + //An exception willl be thrown here, the exception message includes "metric type not match" + MilvusException milvusException = Assert.Throws(() => this.Store.GetNearestMatchesAsync(CollectionName, new[] { 5f, 6f, 7f, 8f, 9f }, limit: 2, withEmbeddings: withEmbeddings).ToEnumerable().ToList()); + + Assert.NotNull(milvusException); + + Assert.Contains("metric type not match", milvusException.Message); + + //Recreate collection with Cosine metric + await this.Store.DeleteCollectionAsync(CollectionName); + await this.Store.CreateCollectionAsync(CollectionName); + await this.InsertSampleDataAsync(); + // There seems to be some race condition where the upserted data above isn't taken into account in the search below and zero results are returned... + await Task.Delay(1000); + + //Search with Ip metric, run correctly + List<(MemoryRecord Record, double SimilarityScore)> cosineResults = + this.Store.GetNearestMatchesAsync(CollectionName, new[] { 5f, 6f, 7f, 8f, 9f }, limit: 2, withEmbeddings: withEmbeddings).ToEnumerable().ToList(); + + Assert.All(cosineResults, t => Assert.True(t.SimilarityScore > 0)); + } + [Fact] public async Task GetNearestMatchesWithMinRelevanceScoreAsync() { From db99d330fb58dc4d586c131183cf34d330663f72 Mon Sep 17 00:00:00 2001 From: ShuaiHua Du Date: Wed, 3 Jul 2024 11:17:37 +0800 Subject: [PATCH 3/3] Flush the milvus collection data after insert sample data, to avoid the race issue. --- .../Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs b/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs index 64edf1280f1b..5fba220a3ad4 100644 --- a/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs +++ b/dotnet/src/IntegrationTests/Connectors/Memory/Milvus/MilvusMemoryStoreTests.cs @@ -228,8 +228,7 @@ public async Task GetNearestMatchesWithMetricTypeAsync(bool withEmbeddings) //Create collection with default, Ip metric await this.Store.CreateCollectionAsync(CollectionName); await this.InsertSampleDataAsync(); - // There seems to be some race condition where the upserted data above isn't taken into account in the search below and zero results are returned... - await Task.Delay(1000); + await this.Store.Client.FlushAsync([CollectionName]); //Search with Ip metric, run correctly List<(MemoryRecord Record, double SimilarityScore)> ipResults = @@ -240,7 +239,7 @@ public async Task GetNearestMatchesWithMetricTypeAsync(bool withEmbeddings) //Set the store to Cosine metric, without recreate collection this.Store = new(this._milvusFixture.Host, vectorSize: 5, port: this._milvusFixture.Port, metricType: SimilarityMetricType.Cosine, consistencyLevel: ConsistencyLevel.Strong); - //An exception willl be thrown here, the exception message includes "metric type not match" + //An exception will be thrown here, the exception message includes "metric type not match" MilvusException milvusException = Assert.Throws(() => this.Store.GetNearestMatchesAsync(CollectionName, new[] { 5f, 6f, 7f, 8f, 9f }, limit: 2, withEmbeddings: withEmbeddings).ToEnumerable().ToList()); Assert.NotNull(milvusException); @@ -251,8 +250,7 @@ public async Task GetNearestMatchesWithMetricTypeAsync(bool withEmbeddings) await this.Store.DeleteCollectionAsync(CollectionName); await this.Store.CreateCollectionAsync(CollectionName); await this.InsertSampleDataAsync(); - // There seems to be some race condition where the upserted data above isn't taken into account in the search below and zero results are returned... - await Task.Delay(1000); + await this.Store.Client.FlushAsync([CollectionName]); //Search with Ip metric, run correctly List<(MemoryRecord Record, double SimilarityScore)> cosineResults =