-
Notifications
You must be signed in to change notification settings - Fork 831
fix seed #5503
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 seed #5503
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.
Summary of Changes
Hello @tastelikefeet, 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!
This pull request addresses a bug related to how 'thinking budget' is calculated and reported within the SeedTemplate
for large language models. The changes introduce flexibility by allowing the budget to be configured via an environment variable and improve the precision of budget calculations and token usage reporting within generated text.
Highlights
- Environment Variable Support for Thinking Budget: The
get_thinking_budget
function now allows the 'thinking budget' for the LLM to be set via an environment variable namedTHINKING_BUDGET
. If this variable is present, its value will override the dynamic calculation based on message content. - Refined Budget Rounding Logic: The logic for calculating and rounding the thinking budget in
convert_integer_v2
has been refined. The comparison operators were changed from exclusive less-than (<
) to inclusive less-than-or-equal-to (<=
), which provides more accurate rounding for specific budget thresholds. - Improved Token Budget Marker Accuracy: The
insert_budget_markers
function, responsible for adding budget reflection markers into text, now calculates the 'used tokens' and 'remaining budget' more precisely based on the actual token count at the insertion point, rather than relying solely on fixed interval multiples. This provides a more accurate representation of token usage.
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 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
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.
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
This pull request introduces fixes to the seed template. It makes the thinking budget configurable via an environment variable and corrects the boundary conditions for budget calculation. It also improves the accuracy of the budget usage reporting in the generated markers. My review includes a high-severity comment to handle potential ValueError
when parsing the environment variable to prevent crashes, and a medium-severity comment suggesting a refactoring to improve code maintainability.
max_length = thinking_token_len | ||
thinking_budget = os.environ.get('THINKING_BUDGET') | ||
if thinking_budget is not None: | ||
max_length = int(thinking_budget) |
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 conversion of thinking_budget
from a string to an integer using int()
can raise a ValueError
if the environment variable contains a non-integer string. This would cause the program to crash. It's safer to wrap this conversion in a try-except
block to handle potential errors gracefully. Setting max_length
to 0 on failure is a reasonable fallback.
max_length = int(thinking_budget) | |
try: | |
max_length = int(thinking_budget) | |
except ValueError: | |
max_length = 0 # Fallback to 0 if the environment variable is not a valid integer. |
if n is None: | ||
return None | ||
elif n <= 0: | ||
return 0 | ||
elif n < 512: | ||
elif n <= 512: | ||
return 512 | ||
elif n < 1024: | ||
elif n <= 1024: | ||
return 1024 | ||
elif n < 2048: | ||
elif n <= 2048: | ||
return 2048 | ||
elif n < 4096: | ||
elif n <= 4096: | ||
return 4096 | ||
elif n < 8192: | ||
elif n <= 8192: | ||
return 8192 | ||
elif n < 16384: | ||
elif n <= 16384: | ||
return 16384 | ||
else: | ||
return n |
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.
This series of elif
statements can be simplified by using a loop over a list of thresholds. This would make the code more concise and easier to maintain if more thresholds are added in the future.
if n is None:
return None
if n <= 0:
return 0
thresholds = [512, 1024, 2048, 4096, 8192, 16384]
for threshold in thresholds:
if n <= threshold:
return threshold
return n
PR type
PR information
Write the detail information belongs to this PR.
Experiment results
Paste your experiment result here(if needed).