-
Notifications
You must be signed in to change notification settings - Fork 612
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: set topP from config #1053
fix: set topP from config #1053
Conversation
Thanks for opening a PR, I think for this to work though, you'll need to update the command/auth with a new flag for topP and link it into the configuration object |
@AlexsJones thanks for the comment. That flag is already there in the auth command and if With the edits in this PR, that parameter is properly set from the config if the user provides it. |
You are right, probably we never removed the constant when introducing the topP as a cli argument. Could you please, also add it as an env variable in server's cmd file so we can use it in the operator? |
@arbreezy thanks for reviewing this. I took care of the format and I have also enabled the setting of |
It looks good, after resolving the conflicts I'll approve 👍 |
Signed-off-by: “Guido <muscionig@gmail.com>
Signed-off-by: “Guido <muscionig@gmail.com>
Signed-off-by: “Guido <muscionig@gmail.com>
363ea76
to
b8742e9
Compare
@arbreezy I rebased off of |
Closes #1052
📑 Description
This PR allows for setting topP from the config yaml for the
openai
provider.✅ Checks
ℹ Additional Information
I took the liberty of opening both the issue and the PR fixing it, please let me know if I should follow any other process.