-
Notifications
You must be signed in to change notification settings - Fork 2.9k
feat: Update model name to gemini-embedding-001 #10098
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @Feiyang1, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request. This PR, authored by Feiyang1, aims to update the sample code for predicting text embeddings to use the newer gemini-embedding-001
model instead of the older text-embedding-005
. It also adjusts the embedding dimension parameter passed to the prediction function, likely to align with the new model's expected output size.
Highlights
- Model Update: The primary change is updating the model name used in the
PredictTextEmbeddingsSample.java
file fromtext-embedding-005
togemini-embedding-001
. - Embedding Dimension Adjustment: The sample code now requests an embedding dimension of
3072
instead of256
, which is likely the standard or recommended dimension for thegemini-embedding-001
model.
Changelog
- aiplatform/src/main/java/aiplatform/PredictTextEmbeddingsSample.java
- Updated the default model name from
text-embedding-005
togemini-embedding-001
on line 44. - Changed the requested embedding dimension from
256
to3072
on line 51.
- Updated the default model name from
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Code needs update,
New model name, dimension,
Embeddings flow now.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
Thank you for submitting this pull request! The change updates the sample code to use the gemini-embedding-001
model, which is a valuable update. The core change of updating the model name and output dimensionality is straightforward.
However, I've identified a potential issue regarding the documentation link provided in the sample code, which currently points to documentation for older models. Additionally, the existing test file does not seem to cover the new model and dimensionality. Please see the specific comments below for details.
Overall, the change is a good step, but addressing the documentation and testing aspects will improve the quality and usability of the sample.
Summary of Findings
- Documentation/Sample Discrepancy: The sample code uses the
gemini-embedding-001
model with output dimensionality 3072, but the linked documentation refers to older models and dimensions. The documentation link or comments should be updated to match the sample for clarity and correctness. - Test File Relevance (Not commented on directly): The test file
PredictTextEmbeddingsSampleTest.java
still uses the old model (text-embedding-005
) and a different output dimensionality (5). This test might need to be updated or a new test added for thegemini-embedding-001
model to ensure the sample works correctly with the new parameters. (Note: I could not comment on this directly as the file was not changed in the PR).
Merge Readiness
This pull request introduces a functional update to use a newer model. However, there is a high-severity issue regarding the discrepancy between the sample code's parameters and the linked documentation, which needs to be addressed to ensure users can correctly use the sample. Additionally, the existing test does not cover the new model. Therefore, I recommend addressing these points before merging. I am unable to approve this pull request; please have other reviewers approve it once the necessary changes are made.
String model = "gemini-embedding-001"; | ||
predictTextEmbeddings( | ||
endpoint, | ||
project, | ||
model, | ||
List.of("banana bread?", "banana muffins?"), | ||
"QUESTION_ANSWERING", | ||
OptionalInt.of(256)); | ||
OptionalInt.of(3072)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sample code is updated to use the gemini-embedding-001
model with an output dimensionality of 3072. However, the comment on line 41 links to documentation that primarily discusses older models (text-embedding-004
, text-embedding-005
) and different output dimensions (768, 256). This discrepancy could be confusing for users trying to understand the correct parameters for the gemini-embedding-001
model.
Could you please update the documentation link or the surrounding comments to accurately reflect the gemini-embedding-001
model and its expected output dimensionality? This ensures users have access to the correct and relevant information for this specific sample.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
documentation link at line 41 has already been updated to include gemini-embedding-001
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, @Feiyang1. If the documentation link on line 41 now accurately reflects the gemini-embedding-001
model and its expected output dimensionality (3072), then this addresses the concern raised.
Description
Fixes #
Note: Before submitting a pull request, please open an issue for discussion if you are not associated with Google.
Checklist
pom.xml
parent set to latestshared-configuration
mvn clean verify
requiredmvn -P lint checkstyle:check
requiredmvn -P lint clean compile pmd:cpd-check spotbugs:check
advisory only