You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The 'omitempty' option of the request structs should be removed. This is because it generates an incorrect request when a parameter is 0. For example, a request with the temperate field set to '0' will actually return a response as if the field had been set to one. This is because the go JSON parser does not differentiate between a 0 value and no value for float32, so Openai will receive a request without a temperate field which then defaults to a temperature value of 1.
liushuangls, songohannyc, sd2k, nanmu42 and boratanrikulu
changed the title [-]`omitempty` field of struct will generate incorrect request when parameter is 0.[/-][+]`omitempty` option of request struct will generate incorrect request when parameter is 0.[/+]on Aug 7, 2021
Looking at CompletionRequest in completion.go it seems that OpenAPI's default parameter values do not match Go's zero-values:
typeCompletionRequeststruct {
Promptstring`json:"prompt,omitempty"`MaxTokensint`json:"max_tokens,omitempty"`// OpenAPI defaults to 16Temperaturefloat32`json:"temperature,omitempty"`// OpenAPI defaults to 1TopPfloat32`json:"top_p,omitempty"`// OpenAPI defaults to 1Nint`json:"n,omitempty"`// OpenAPI defaults to 1LogProbsint`json:"logprobs,omitempty"`Model*string`json:"model,omitempty"`Echobool`json:"echo,omitempty"`Stop []string`json:"stop,omitempty"`PresencePenaltyfloat32`json:"presence_penalty,omitempty"`// OpenAPI defaults to 0FrequencyPenaltyfloat32`json:"frequency_penalty,omitempty"`// OpenAPI defaults to 0BestOfint`json:"best_of,omitempty"`// OpenAPI defaults to 1
}
Perhaps it would be best to make a constructor function for the struct then? Or at least documenting the quirk. As of right now, calling the API with parameters generated from the playground will have (sometimes significantly) different results as calling the exact same parameters from a similar API in another language.
Hello,
Wanted to mention that I've just run into this problem as well.
If we don't want to get rid of the omitempty (because of different defaults), could we make the types for fields like Temperature a pointer (e.g. *float32)? That way, it will be omitted if you don't set it but it will be marshaled as 0 if you specifically set it to 0.
From reading about other people's usage of Temperature and TopP, it seems like the most common values for them are 1, 0.5, and 0. So 0 isn't an edge case. For now, I'm setting Temperature to a tiny amount to mimic 0, but that seems a little hacky.
Kardbord, EthanBlackburn, adayNU, hugorut, liaoyuming and 14 more
Hi, just another idea to resolve this: when marshalling and unmarshalling, we can set default values other than 0 by building a custom 'UnmarshalJSON' for an Options struct type.
How do you like an option similar to how it is done in the SQL package from the GO standard library for nullable fields in the database? They just added a separate structure for such properties - NullString, NullFloat64 and etc. (example: https://cs.opensource.google/go/go/+/master:src/database/sql/sql.go;l=319).
If valid is false, then send the default value for the API - that is, 1, otherwise explicitly send the value from the corresponding property.
SentientRamen12, rayhankinan and sollyuckoSentientRamen12 and rayhankinan
Activity
[-]`omitempty` field of struct will generate incorrect request when parameter is 0.[/-][+]`omitempty` option of request struct will generate incorrect request when parameter is 0.[/+]sashabaranov commentedon Aug 8, 2021
Thank you for the issue! So, with
omitempty
we have:1e-45 ≈ math.SmallestNonzeroFloat32
Without
omitempty
:I would check whether OpenAPI defaults match Go's zero-values. In case they do I think it is best to remove
omitempty
.sashabaranov commentedon Aug 8, 2021
Looking at
CompletionRequest
in completion.go it seems that OpenAPI's default parameter values do not match Go's zero-values:ArthurAmes commentedon Aug 8, 2021
Perhaps it would be best to make a constructor function for the struct then? Or at least documenting the quirk. As of right now, calling the API with parameters generated from the playground will have (sometimes significantly) different results as calling the exact same parameters from a similar API in another language.
sashabaranov commentedon Aug 9, 2021
It seems that playground's default parameters match API ones, except for
max_tokens
. Which parameters did you tune in order to get proper result?neilmadsen commentedon Nov 1, 2021
Hello,
Wanted to mention that I've just run into this problem as well.
If we don't want to get rid of the omitempty (because of different defaults), could we make the types for fields like Temperature a pointer (e.g.
*float32
)? That way, it will be omitted if you don't set it but it will be marshaled as 0 if you specifically set it to 0.From reading about other people's usage of Temperature and TopP, it seems like the most common values for them are 1, 0.5, and 0. So 0 isn't an edge case. For now, I'm setting Temperature to a tiny amount to mimic 0, but that seems a little hacky.
Refactor library (#1)
SentientRamen12 commentedon Mar 26, 2023
Hi, just another idea to resolve this: when marshalling and unmarshalling, we can set default values other than 0 by building a custom 'UnmarshalJSON' for an Options struct type.
reference: https://eli.thegreenplace.net/2020/optional-json-fields-in-go/
fussraider commentedon Mar 26, 2023
How do you like an option similar to how it is done in the SQL package from the GO standard library for nullable fields in the database? They just added a separate structure for such properties - NullString, NullFloat64 and etc. (example: https://cs.opensource.google/go/go/+/master:src/database/sql/sql.go;l=319).
If valid is false, then send the default value for the API - that is, 1, otherwise explicitly send the value from the corresponding property.
SentientRamen12 commentedon Mar 27, 2023
@fussraider that can also be done, we just need to make sure the marshalling and unmarshalling handles it cleanly.
26 remaining items