-
Notifications
You must be signed in to change notification settings - Fork 2k
Refactor Usage handling #2108
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
Refactor Usage handling #2108
Conversation
- Remove model specific Usage implementations
- Add `Object getNativeUsage()` to Usage interface
- This will allow the model specific Usage data to be returned
- At the client side, client needs to cast the return type of `getNativeUsage` into the corresponding Usage returned by the model API
- Rename `generationTokens` to `completionTokens`
- Since `completion` token name is more common among the models, renaming generation tokens into completion tokens
- Change the prompt, completion and total token return types to Integer
- Use DefaultUsage for most of the model specific usage handling
- When initializing set the native usage to the model specific usage type
Resolves spring-projects#1407
482c07a to
42fd8f0
Compare
| public DefaultUsage(@JsonProperty("promptTokens") Integer promptTokens, | ||
| @JsonProperty("completionTokens") Integer completionTokens, | ||
| @JsonProperty("totalTokens") Integer totalTokens) { | ||
| this(promptTokens, completionTokens, totalTokens, null); |
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 native usage argument should not be null but Map.of, even though it is handled later.
| DefaultUsage that = (DefaultUsage) o; | ||
| return Objects.equals(this.promptTokens, that.promptTokens) | ||
| && Objects.equals(this.generationTokens, that.generationTokens) | ||
| && Objects.equals(this.completionTokens, that.completionTokens) |
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.
equals should also include the nativeUsage field in the calculate, as well as for hascode/tostring
| private final Long generationTokens; | ||
|
|
||
| private final Long totalTokens; | ||
| private final int totalTokens; |
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.
should be Integer no?
| * @author Ilayaperumal Gopinathan | ||
| * @since 0.7.0 | ||
| */ | ||
| public class EmptyUsage implements Usage { |
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.
we should think about getting rid of this and use 'new DefaultUsage(0,0)
|
Added support to ser/deser the native usage object We should discuss getting rid of EmtpyUsage, can call DefaultUsage(0, 0) mergedin 4b64aa0 |
Object getNativeUsage()to Usage interfacegetNativeUsageinto the corresponding Usage returned by the model APIgenerationTokenstocompletionTokenscompletiontoken name is more common among the models, renaming generation tokens into completion tokens