fix: include IMAGE token in token count (counting as text tokens) #14286
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
wouldn't it be better to track it separately (similar to text and audio tokens)
so that if there is a price change in future models, the code 'just works' ? @tremlin
There was a problem hiding this comment.
@krrishdholakia Thank you for your feedback.
I've also considered this (counting image tokens separately), but I wasn't sure what the best approach was.
What do you think about the following:
- introduce a new pricing key option
input_cost_per_image_tokeninmodel_prices_and_context_window.json - in
litellm/litellm_core_utils/llm_cost_calc/utils.pychange code ofcalculate_cost_componentandgeneric_cost_per_tokensuch that- when the prompt_tokens_details contain an
image_token_count(also to be added), then first it is being checked whether the model pricing contains the newinput_cost_per_image_tokenoption. - If it is does not contain the new option, then use
input_cost_per_tokenas default for cost calculation.
- when the prompt_tokens_details contain an
Alternatively, we could also decide to only count image tokens separately, but use always input_cost_per_token for cost calculation. This would avoid the introduction of a new pricing parameter that is not needed at the moment.
Both approaches have a sane default, which I think is important because it's hard to get pricing parameters right when there are too many options.
There was a problem hiding this comment.
input_cost_per_image_token
yes this makes sense, it follows the existing patterns as well! please update and bump me once it's ready for review
thank you for your help!
There was a problem hiding this comment.
I've finished my implementation and I am looking forward to your feedback!
I will be traveling starting next week, so I might not respond very quickly.
Some CI tests are failing, but all of these seem unrelated to my changes. When I run the unit tests locally, I get:
============================================================= short test summary info =============================================================
FAILED tests/test_litellm/llms/databricks/chat/test_databricks_chat_transformation.py::test_convert_anthropic_tool_to_databricks_tool_with_description - TypeError: litellm.types.llms.databricks.DatabricksFunction() got multiple values for keyword argument 'name'
FAILED tests/test_litellm/llms/databricks/chat/test_databricks_chat_transformation.py::test_convert_anthropic_tool_to_databricks_tool_without_description - TypeError: litellm.types.llms.databricks.DatabricksFunction() got multiple values for keyword argument 'name'
FAILED tests/test_litellm/litellm_core_utils/test_duration_parser.py::TestStandardizedResetTime::test_timezone_handling - zoneinfo._common.ZoneInfoNotFoundError: 'No time zone found with key US/Eastern'
===================================== 3 failed, 2948 passed, 15 skipped, 2830 warnings in 2068.82s (0:34:28) ======================================
tremlin
force-pushed
the
fix/gemini-cost-image-count
branch
from
f9dfc2c to
191883f
Compare
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
|
same issue, image tokens will not be included. |
3 participants
Title
Fix missing IMAGE token count for Gemini cost calculation
Relevant issues
fixes #14285
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unitOne unit test fails, but this is unrelated to this PR.
Type
🐛 Bug Fix
Changes
Counts IMAGE tokens as TEXT tokens because this seems to be the best fix.
Image input for Gemini is usually billed by token with the same rate as text.