feat: Add openai embedding #36366

junjiejiangjjj · 2024-09-19T09:51:47Z

#35856

sre-ci-robot · 2024-09-19T09:51:58Z

Welcome @junjiejiangjjj! It looks like this is your first PR to milvus-io/milvus 🎉

mergify · 2024-09-19T09:53:40Z

@junjiejiangjjj Please associate the related issue to the body of your Pull Request. (eg. “issue: #”)

junjiejiangjjj · 2024-12-13T06:02:30Z

rerun go-sdk

mergify · 2024-12-17T10:20:41Z

@junjiejiangjjj E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-17T10:22:08Z

@junjiejiangjjj go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-18T03:20:53Z

@junjiejiangjjj go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-18T09:44:26Z

@junjiejiangjjj E2e jenkins job failed, comment /run-cpu-e2e can trigger the job again.

mergify · 2024-12-19T08:22:29Z

@junjiejiangjjj cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

Signed-off-by: junjie.jiang <[email protected]>

Signed-off-by: junjiejiangjjj <[email protected]>

Signed-off-by: junjie.jiang <[email protected]>

mergify · 2024-12-26T09:16:51Z

@junjiejiangjjj go-sdk check failed, comment rerun go-sdk can trigger the job again.

mergify · 2024-12-26T09:20:22Z

@junjiejiangjjj cpp-unit-test check failed, comment rerun cpp-unit-test can trigger the job again.

zhengbuqian

I also see several comments from last review nor addressed neither replied, please take a look. Thanks!

zhengbuqian · 2024-12-17T03:16:20Z

internal/models/openai/openai_embedding.go

+			apiKey: apiKey,
+			url:    url,
+		},
+		apiVersion: "2024-06-01",


does it make sense to pass in this apiVersion instead of hard coding?

based on https://learn.microsoft.com/en-us/azure/ai-services/openai/api-version-deprecation seems there is little breaking change introduced to the API, mostly new features.

New features brought by the new version generally require code adaptation, so it is not very meaningful to open the configuration of apiVersion.

zhengbuqian · 2024-12-26T08:13:43Z

internal/proxy/task_search.go

@@ -416,14 +417,24 @@ func (t *searchTask) initAdvancedSearchRequest(ctx context.Context) error {
 			zap.Stringer("plan", plan)) // may be very large if large term passed.
 	}

+	var err error
+	if function.HasFunctions(t.schema.CollectionSchema.Functions, []int64{}) {


zhengbuqian · 2024-12-26T08:14:58Z

internal/storage/utils.go

+
+// TODO: unify the function implementation, storage/utils.go & proxy/util.go
+func IsBM25FunctionOutputField(field *schemapb.FieldSchema) bool {
+	return field.GetIsFunctionOutput() && field.GetDataType() == schemapb.DataType_SparseFloatVector


this is not true? other models can also have sparse float vector output

There is no problem right now. After the implementation of bm25 and the model function is unified, the relevant code will not be needed.

zhengbuqian · 2024-12-26T14:43:27Z

internal/models/utils/embedding_util.go

+		return nil, err
+	}
+
+	if resp.StatusCode != 200 {


Suggested change

if resp.StatusCode != 200 {

if resp.StatusCode != http.StatusOK {

zhengbuqian · 2024-12-26T14:47:15Z

internal/models/ali/ali_dashscope_text_embedding.go

+	OutputType string `json:"output_type,omitempty"`
+}
+
+type EmbeddingRequest struct {


EmbeddingRequest can be private? by lowercase the first letter embeddingRequest. I did a quick search and find no usage of this class outside this file. correct me IIW. This comment applies to many other structs in this and other files.

Only used in test files under milvus/internal/util/function/

zhengbuqian · 2024-12-26T15:44:40Z

internal/util/function/text_embedding_function.go

+	}
+
+	if inputs[0].Type != schemapb.DataType_VarChar {
+		return nil, fmt.Errorf("Text embedding only supports varchar field, the input is not varchar")


Text embedding only supports varchar field as input field, but got %s

zhengbuqian · 2024-12-26T15:47:08Z

internal/util/function/text_embedding_function.go

+
+func (runner *TextEmebddingFunction) ProcessBulkInsert(inputs []storage.FieldData) (map[storage.FieldID]storage.FieldData, error) {
+	if len(inputs) != 1 {
+		return nil, fmt.Errorf("OpenAIEmbedding function only receives one input, bug got [%d]", len(inputs))


zhengbuqian · 2024-12-26T15:49:12Z

internal/util/function/openai_embedding_provider.go

+			dim, err = strconv.ParseInt(param.Value, 10, 64)
+			if err != nil {
+				return nil, fmt.Errorf("dim [%s] is not int", param.Value)
+			}
+
+			if dim != 0 && dim != fieldDim {
+				return nil, fmt.Errorf("Field %s's dim is [%d], but embeding's dim is [%d]", fieldSchema.Name, fieldDim, dim)
+			}


extract this dim parsing code into a function in common.go

zhengbuqian · 2024-12-26T15:52:19Z

internal/models/openai/openai_embedding.go

+		return nil, err
+	}
+
+	if timeoutSec <= 0 {


consider make default timeoutSec a constant in common.go? I see this number and default retry times 3 used in several places.

zhengbuqian · 2024-12-26T15:55:39Z

internal/models/openai/openai_embedding.go

+	}
+}
+
+func (c *OpenAIEmbeddingClient) Embedding(modelName string, texts []string, dim int, user string, timeoutSec time.Duration) (*EmbeddingResponse, error) {


this function is largely duplicate with func (c *AzureOpenAIEmbeddingClient) Embedding, can we try share the same logic?

sre-ci-robot added the do-not-merge/work-in-progress Don't merge even CI passed. label Sep 19, 2024

sre-ci-robot added the size/L Denotes a PR that changes 100-499 lines. label Sep 19, 2024

sre-ci-robot requested review from czs007 and zwd1208 September 19, 2024 09:52

mergify bot added dco-passed DCO check passed. kind/feature Issues related to feature request from users labels Sep 19, 2024

mergify bot added the do-not-merge/missing-related-issue label Sep 19, 2024

junjiejiangjjj mentioned this pull request Sep 19, 2024

[Feature]: support Data-in Data-out #35856

Open

1 task

junjiejiangjjj force-pushed the embedding branch 3 times, most recently from fd701cd to 5407f22 Compare September 20, 2024 03:03

sre-ci-robot added size/XL Denotes a PR that changes 500-999 lines. and removed size/L Denotes a PR that changes 100-499 lines. labels Sep 23, 2024

mergify bot removed the do-not-merge/missing-related-issue label Sep 24, 2024

junjiejiangjjj force-pushed the embedding branch 6 times, most recently from 200bc3b to ed7d332 Compare September 26, 2024 07:25

sre-ci-robot added size/XXL Denotes a PR that changes 1000+ lines. and removed size/XL Denotes a PR that changes 500-999 lines. labels Sep 26, 2024

junjiejiangjjj force-pushed the embedding branch 2 times, most recently from 38243dd to 0d0286e Compare October 8, 2024 08:26

sre-ci-robot added the area/dependency Pull requests that update a dependency file label Oct 8, 2024

junjiejiangjjj force-pushed the embedding branch 3 times, most recently from 0f30960 to 7950316 Compare October 11, 2024 03:53

junjiejiangjjj force-pushed the embedding branch from dc9f47d to aa4932c Compare December 17, 2024 10:01

junjiejiangjjj force-pushed the embedding branch from aa4932c to e7e5e7c Compare December 18, 2024 02:19

junjiejiangjjj force-pushed the embedding branch from 67e5b51 to 6334590 Compare December 19, 2024 07:41

junjiejiangjjj added 17 commits December 26, 2024 15:53

Add openai embedding client

9705f0c

Signed-off-by: junjie.jiang <[email protected]>

Add embedding runner

b117d64

Signed-off-by: junjie.jiang <[email protected]>

Add function executor

4c2baae

Signed-off-by: junjie.jiang <[email protected]>

Insert & Upsert support functions

542c249

Signed-off-by: junjie.jiang <[email protected]>

Search supports embedding function

4587572

Signed-off-by: junjie.jiang <[email protected]>

Add search embedding test

c01d61c

Signed-off-by: junjie.jiang <[email protected]>

Support bulkinsert

a5776f3

Signed-off-by: junjie.jiang <[email protected]>

Use TextEmbedding function

5eea78f

Signed-off-by: junjie.jiang <[email protected]>

update

df07a06

Signed-off-by: junjiejiangjjj <[email protected]>

Add bedrock, azure, ali text embedding

ecc4ad8

Signed-off-by: junjie.jiang <[email protected]>

Update go mod

c459d41

Signed-off-by: junjie.jiang <[email protected]>

Polish error infos

9ff9419

Signed-off-by: junjie.jiang <[email protected]>

Reuse http client

0bcc13a

Signed-off-by: junjie.jiang <[email protected]>

Add vertexai

2692fea

Signed-off-by: junjie.jiang <[email protected]>

Add function check

5c0f58b

Signed-off-by: junjie.jiang <[email protected]>

Adjust dashscope model batch

8c62771

Signed-off-by: junjie.jiang <[email protected]>

Fix confict

b482b3c

Signed-off-by: junjie.jiang <[email protected]>

junjiejiangjjj force-pushed the embedding branch from 477d85e to b482b3c Compare December 26, 2024 08:44

zhengbuqian reviewed Dec 26, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add openai embedding #36366

feat: Add openai embedding #36366

junjiejiangjjj commented Sep 19, 2024 •

edited

Loading

sre-ci-robot commented Sep 19, 2024

mergify bot commented Sep 19, 2024

junjiejiangjjj commented Dec 13, 2024

mergify bot commented Dec 17, 2024

mergify bot commented Dec 17, 2024

mergify bot commented Dec 18, 2024

mergify bot commented Dec 18, 2024

mergify bot commented Dec 19, 2024

mergify bot commented Dec 26, 2024

mergify bot commented Dec 26, 2024

zhengbuqian left a comment

zhengbuqian Dec 17, 2024

junjiejiangjjj Dec 27, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

junjiejiangjjj Dec 27, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

junjiejiangjjj Dec 27, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

zhengbuqian Dec 26, 2024

	if resp.StatusCode != 200 {
	if resp.StatusCode != http.StatusOK {

feat: Add openai embedding #36366

Are you sure you want to change the base?

feat: Add openai embedding #36366

Conversation

junjiejiangjjj commented Sep 19, 2024 • edited Loading

sre-ci-robot commented Sep 19, 2024

mergify bot commented Sep 19, 2024

junjiejiangjjj commented Dec 13, 2024

mergify bot commented Dec 17, 2024

mergify bot commented Dec 17, 2024

mergify bot commented Dec 18, 2024

mergify bot commented Dec 18, 2024

mergify bot commented Dec 19, 2024

mergify bot commented Dec 26, 2024

mergify bot commented Dec 26, 2024

zhengbuqian left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

junjiejiangjjj commented Sep 19, 2024 •

edited

Loading