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: remove the num_template_per_step param in all notebooks #1574

Merged
merged 5 commits into from
Dec 24, 2024

Conversation

want-to-be-relaxed
Copy link
Contributor

Description

Remove the num_template_eval_per_step parameter in all the notebooks.

Thank you for opening a Pull Request!
Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Follow the CONTRIBUTING Guide.
  • You are listed as the author in your notebook or README file.
    • Your account is listed in CODEOWNERS for the file(s).
  • Make your Pull Request title in the https://www.conventionalcommits.org/ specification.
  • Ensure the tests and linter pass (Run nox -s format from the repository root to format).
  • Appropriate docs were updated (if necessary)

@want-to-be-relaxed want-to-be-relaxed requested a review from a team as a code owner December 23, 2024 20:27
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

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 @want-to-be-relaxed and reviewers,

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

This PR removes the num_template_eval_per_step parameter from several notebooks within the generative-ai repository. The intent, as stated in the PR description, is to simply remove this parameter.

Here's a breakdown of the changes:

  • Files Affected:

    • gemini/prompts/prompt_optimizer/vertex_ai_prompt_optimizer_sdk.ipynb (line 924 removed)
    • gemini/prompts/prompt_optimizer/vertex_ai_prompt_optimizer_sdk_custom_metric.ipynb (line 1167 removed)
    • gemini/prompts/prompt_optimizer/vertex_ai_prompt_optimizer_sdk_long_prompt_optimization.ipynb (line 828 removed)
    • gemini/prompts/prompt_optimizer/vertex_ai_prompt_optimizer_sdk_tool_calling.ipynb (line 1146 removed)
    • gemini/prompts/prompt_optimizer/vertex_ai_prompt_optimizer_ui.ipynb (lines 274, 368 removed; line 297-299 simplified)
  • Change Summary: The num_template_eval_per_step parameter, which controlled the number of system instructions evaluated per step in prompt optimization, has been removed from all specified notebooks. The code changes reflect this removal. In vertex_ai_prompt_optimizer_ui.ipynb, some minor code cleanup was also performed around the RESPONSE_MIME_TYPE parameter.

I'll provide a more detailed review shortly, but this should give you a good overview of the changes.

And now, a little haiku to lighten the mood:

Code flows like a stream,
Removing params, clean and bright,
Review's gentle breeze.

Best,
Gemini

Copy link
Contributor

Gemini encountered an error creating the review. You can try again by commenting @code-review-assist review.

Copy link
Contributor

@inardini inardini left a comment

Choose a reason for hiding this comment

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

LGTM

@inardini inardini merged commit 818b589 into GoogleCloudPlatform:main Dec 24, 2024
5 checks passed
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.

2 participants