-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Made the cost info easier to read #2356
Conversation
@microsoft-github-policy-service agree |
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.
Thanks. I have some suggestions.
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.
Please check all places that uses gather_usage_summary
, there should be a notebook that also uses this function?
@kevin666aa got stuck at a point, if you don't mind, could you please ping me up at discord. username: |
Sure! |
How's this PR going? I'll need to address this problem asap. I may make a PR myself if there is no update with this PR by tomorrow. |
would appreciate if you can help me with some more information. @sonichi @kevin666aa |
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.
updated the doc string as proposed.
@kevin666aa could you review again? The PR is blocked by the change request. |
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.
LGTM!
Looks good to me too! |
* gather_usage_summary has been updated * updated cost info to 'usage_including_cached_inference' and 'usage_excluding_cached_inference' * fix: pre-commit formatting for cost_info * improved cost explanation and doc * improved cost info doc * include - exclude --------- Co-authored-by: Chi Wang <[email protected]>
Why are these changes needed?
gathe_usage_summary
to return a dictionary containing two dictionaries:total_usage_summary
andactual_usage_summary
, instead of returning a tuple.Related issue number
Closes #2336
Checks