Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: textembedding gecko 003 change from latest #1572

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bgdnvk
Copy link

@bgdnvk bgdnvk commented Dec 21, 2024

Description

As described in the issue 1571 the gecko version can't load if it's specified as the latest, it's also not recommended by the documentation, I've changed it to the version 003.

Fixes ##1571 🦕

@bgdnvk bgdnvk requested a review from a team as a code owner December 21, 2024 21:53
Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi team,

I'm currently reviewing this pull request and will have a full review ready in a few minutes. In the meantime, here's a quick summary to get everyone up to speed:

This PR, submitted by bgdnvk, addresses issue #1571. The goal is to fix a problem where the textembedding-gecko model failed to load when specifying the latest version. The documentation also discourages using latest, so this change makes the fix more robust.

Here's a breakdown of the changes:

  • File Changed: gemini/use-cases/retrieval-augmented-generation/utils/intro_multimodal_rag_utils.py
  • Specific Change: Line 23 of intro_multimodal_rag_utils.py was modified. The text embedding model instantiation was updated to use a specific version (textembedding-gecko@003) instead of textembedding-gecko@latest.

Before:

text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@latest")

After:

text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@003")

This change ensures that the code uses a known working version of the textembedding-gecko model, improving the reliability and maintainability of the code. No other files were modified.


And now, a little haiku to lighten the mood:

Gecko's version fixed,
Stable code, a joyful sight,
No more loading woes.

Copy link
Contributor

@code-review-assist code-review-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change replaces the "latest" tag for the textembedding-gecko model with the specific version 003. This is a good practice for reproducibility and avoids potential issues with breaking changes in future versions of the model. Pinning the version ensures that the code will continue to work as expected, even if the "latest" tag is updated to a newer, potentially incompatible version. The pull request description clearly explains the reason for the change and links to the related issue, which is helpful for context and traceability.

I studied the codebase to understand the project's coding style and didn't find any explicit style guide, so I'm defaulting to the Google Python Style Guide for my suggestions.

Overall, this is a small but important change that improves the reliability and maintainability of the code.

@@ -20,7 +20,7 @@
from vertexai.vision_models import Image as vision_model_Image
from vertexai.vision_models import MultiModalEmbeddingModel

text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@latest")
text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@003")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low

Since you've pinned the textembedding-gecko model version to 003, it's a good idea to add a comment explaining the reason for choosing this specific version. This will help future maintainers understand the decision and avoid accidentally upgrading to a newer version that might introduce breaking changes or performance regressions. For example, you could mention if this version was chosen for its performance, specific features, or compatibility with other parts of the system. Referencing the documentation or any relevant benchmarks that support the choice of version 003 would also be beneficial. I'm recommending this change to adhere to the Google Python Style Guide, which emphasizes clear and comprehensive documentation.

Suggested change
text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@003")
text_embedding_model = TextEmbeddingModel.from_pretrained("textembedding-gecko@003") # Version 003 chosen for compatibility with issue #1571. See https://github.com/GoogleCloudPlatform/generative-ai/issues/1571

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant