feat(vertexai): Elastic Text-Embedding Model demo. #4127
Conversation
product-auto-label
bot
added
the
samples
kewent
force-pushed
the
text-embedding-google_io_2024_new_model
branch
from
f5e6632
to
20f37c5
Compare
kewent
changed the title
grayside
changed the title
|
This PR is modifying a sample that's currently in conflict with https://github.com/GoogleCloudPlatform/golang-samples/blob/main/aiplatform/text-embeddings/embeddings.go. One of these samples should be removed or the region tag changed. |
aiplatform/snippets/embeddings.go
Outdated
| apiEndpoint, project, model string, texts []string, | ||
| task string, customOutputDimensionality *int) ([][]float32, error) { |
There was a problem hiding this comment.
SG, I'll hard-coded some of them and only leave project and texts.
aiplatform/snippets/embeddings.go
Outdated
| apiEndpoint, project, model string, texts []string, | ||
| task string, customOutputDimensionality *int) ([][]float32, error) { |
There was a problem hiding this comment.
From https://googlecloudplatform.github.io/samples-style-guide/#minimal-arguments, we should be using minimal arguments.
blocker: apiEndpoint is generally not specified in samples and should be removed. Looking at the code, it seems like maybe the problem is there's a missing method in the library to facilitate text embedding? If so we can make this not-blocker.
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| if len(embeddings) != len(texts) || len(embeddings[0]) != 768 { | ||
| t.Errorf("len(embeddings), len(embeddings[0]) = %d, %d, want %d, 768", len(embeddings), len(embeddings[0]), len(texts)) | ||
| if len(embeddings) != len(texts) || len(embeddings[0]) != dimensionality { |
There was a problem hiding this comment.
suggestion: consider improving readability by extracting repeating len calls to 'got' variables.
aiplatform/snippets/embeddings.go
Outdated
| @@ -28,7 +28,8 @@ import ( | |||
| ) | |||
|
|
|||
| func embedTexts( | |||
| apiEndpoint, project, model string, texts []string, task string) ([][]float32, error) { | |||
| apiEndpoint, project, model string, texts []string, | |||
| task string, customOutputDimensionality *int) ([][]float32, error) { | |||
There was a problem hiding this comment.
From https://googlecloudplatform.github.io/samples-style-guide/#minimal-arguments, we should be using minimal arguments.
suggestion: Remove customOutputDimensionality as a parameter in favor of hard-coding in the function body.
|
Here is the summary of changes. You are about to delete 1 region tag.
This comment is generated by snippet-bot.
|
Removed duplicated files under ./text-embeddings/ |
| ctx := context.Background() | ||
|
|
||
| apiEndpoint := "us-central1-aiplatform.googleapis.com:443" |
There was a problem hiding this comment.
blocker: samples do not specify the API Endpoint unless specifically needed for the sample. Setting the region explicitly is typical.
Description
Fixes: http://b/334964454
Checklist
go test -v ./..(see Testing)gofmt(see Formatting)go vet(see Formatting)