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

feat(vertexai): use pointers for GenerationConfig fields #9182

Merged
merged 2 commits into from
Dec 17, 2023
Merged

feat(vertexai): use pointers for GenerationConfig fields #9182

merged 2 commits into from
Dec 17, 2023

Conversation

jba
Copy link
Contributor

@jba jba commented Dec 15, 2023

Make the fields that are pointers in in the GenerationConfig proto be
pointers in the veneer as well.

This fixes a bug with temperature: when it was a float32, we intpreted zero as nil.
This meant it was impossible to set a zero temperature: attempting to do
so would transmit a nil Temperature to the server, which the server
would interpret as the default. We changed the other fields for consistency.

To make it convenient to set these pointer fields, provide a generic Ptr function
as well as a Set method for each field.

Also, remove the model config defaults, which no longer seem to be necessary.

Make the fields that are pointers in in the GenerationConfig proto be
pointers in the veneer as well.

This fixes a bug with temperature: when it was a float32, we intpreted zero as nil.
This meant it was impossible to set a zero temperature: attempting to do
so would transmit a nil Temperature to the server, which the server
would interpret as the default. We changed the other fields for consistency.

To make it convenient to set these pointer fields, provide a generic `Ptr` function
as well as a Set method for each field.

Also, remove the model config defaults, which no longer seem to be necessary.
@jba jba requested a review from a team as a code owner December 15, 2023 21:48
@jba jba requested a review from eliben December 15, 2023 21:48
@jba jba requested a review from a team as a code owner December 15, 2023 21:48
@product-auto-label product-auto-label bot added the size: m Pull request size is medium. label Dec 15, 2023
@jba jba added automerge Merge the pull request once unit tests and other checks pass. breaking change allowed labels Dec 15, 2023
Copy link
Contributor

Merge-on-green attempted to merge your PR for 6 hours, but it was not mergeable because either one of your required status checks failed, one of your required reviews was not approved, or there is a do not merge label. Learn more about your required status checks here: https://help.github.com/en/github/administering-a-repository/enabling-required-status-checks. You can remove and reapply the label to re-run the bot.

@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Dec 16, 2023
@jba jba merged commit 91990c1 into main Dec 17, 2023
13 checks passed
@jba jba deleted the tempptr branch December 17, 2023 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants