-
Notifications
You must be signed in to change notification settings - Fork 116
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 Bedrock token count and IDs for Anthropic models #341
base: main
Are you sure you want to change the base?
Fix Bedrock token count and IDs for Anthropic models #341
Conversation
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.
@michaelnchin
Thanks for submitting these changes. A question about the version checks, and a suggestion to use of warnings rather than logger statements.
libs/aws/langchain_aws/utils.py
Outdated
python_version = sys.version_info | ||
if python_version > (3, 12): | ||
bad_deps.append(f"Python 3.12 or earlier required, found {'.'.join(map(str, python_version[:3]))})") | ||
|
||
bad_anthropic = None | ||
try: | ||
import anthropic | ||
anthropic_version = version.parse(anthropic.__version__) | ||
if anthropic_version > version.parse("0.38.0"): | ||
bad_anthropic = anthropic_version | ||
except ImportError: | ||
bad_anthropic = "none installed" | ||
|
||
bad_httpx = None | ||
try: | ||
import httpx | ||
httpx_version = version.parse(httpx.__version__) | ||
if httpx_version > version.parse("0.27.2"): | ||
bad_httpx = httpx_version | ||
except ImportError: | ||
raise ImportError( | ||
"Could not import anthropic python package. " | ||
"This is needed in order to accurately tokenize the text " | ||
"for anthropic models. Please install it with `pip install anthropic`." | ||
) | ||
bad_httpx = "none installed" | ||
|
||
if bad_anthropic: | ||
bad_deps.append(f"anthropic<=0.38.0 required, found {bad_anthropic}.") | ||
if bad_httpx: | ||
bad_deps.append(f"httpx<=0.27.2 required, found {bad_httpx}.") | ||
|
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.
Curious about these other checks here, would only checking the anthropic version not suffice?
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.
bad_deps.append(f"httpx<=0.27.2 required, found {bad_httpx}.")
The langsmith
chain dependency of langchain-aws
installs the latest httpx
version.
Collecting httpx<1,>=0.23.0 (from langsmith<0.4,>=0.1.125->langchain-core<0.4.0,>=0.3.27->langchain-aws==0.2.11)
Using cached httpx-0.28.1-py3-none-any.whl.metadata (7.1 kB)
However, this is incompatible with anthropic==0.38.0
, which breaks on anything higher than httpx==0.27.2
. Unfortunately, Anthropic 0.38.0 itself does not cap httpx
to the version it requires, and subsequently may break after it picks up the version installed by LangSmith SDK:
Collecting httpx<1,>=0.23.0 (from anthropic==0.38.0)
Using cached httpx-0.28.1-py3-none-any.whl.metadata (7.1 kB)
So, users must install httpx<=0.27.2
to get the Anthropic tokens methods working.
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.
bad_deps.append(f"Python 3.12 or earlier required, found {'.'.join(map(str, python_version[:3]))})")
anthropic==0.38.0
used to be incompatible with Python 3.13 due to issues with a chain dependency (pyo3-ffi
): anthropics/anthropic-sdk-python#718
But pyo3-ffi
was recently updated, and the 3.13 installation appears to be fixed now - will remove this check as it is no longer necessary.
libs/aws/langchain_aws/utils.py
Outdated
|
||
def enforce_stop_tokens(text: str, stop: List[str]) -> str: | ||
"""Cut off the text as soon as any stop words occur.""" | ||
return re.split("|".join(stop), text, maxsplit=1)[0] | ||
|
||
|
||
def _get_anthropic_client() -> Any: | ||
def check_anthropic_tokens_dependencies() -> List[str]: |
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.
nit: Would anthropic_tokens_supported be a better name here?
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 this 👍
bad_deps = check_anthropic_tokens_dependencies() | ||
if not bad_deps: | ||
return get_num_tokens_anthropic(text) | ||
else: | ||
logger.debug( | ||
"Falling back to default token counting due to incompatible/missing Anthropic dependencies:" | ||
) | ||
for x in bad_deps: | ||
logger.debug(x) |
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.
Would this cause duplicate logger messages, as super().get_num_tokens(text)
ends up calling the get_token_ids
method. Also, adding a logger message would just fall under the radar for most users, unless they have the debug logging switched on, should we rather use a warning?
import warnings
if self._model_is_anthropic and not self.custom_get_token_ids:
warnings.warn(
"Falling back to default token counting due to incompatible anthropic count_tokens API. "
"For anthropic versions > 0.38.0, it is recommended to provide a custom_get_token_ids "
"method to the chat model class that implements the appropriate tokenizer for Anthropic. "
"Alternately, you can implement your own token counter method using the ChatAnthropic "
"or AnthropicLLM classes."
)
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 for the catch - removed the warning from get_num_tokens
, and updated the message as suggested.
Context in issue: #314
The
get_num_tokens()
andget_token_ids()
methods ofBedrockLLM
andChatBedrock
currently fail when used with Anthropic models andanthropic>=0.39.0
installed.In such scenarios, this PR fixes
get_num_tokens()
andget_token_ids()
by falling back to the base class implementations inBaseLanguageModel
and inBaseChatModel
, which use the HuggingFace GPT2 Tokenizer.If
anthropic<=0.38.0
(and other requirements) are present instead, the Anthropic SDK token methods will continue to be used as normal.Note about tokenizer accuracy:
The GPT2 and Anthropic SDK tokenizers (see here) both may produce inaccurate token estimates for Claude 3 and 3.5 models. To obtain a more accurate estimate, Anthropic recommends using the new Count Message Tokens API. For example:
As another alternative, you can implement your own token counter method, and pass this using
custom_get_token_ids
when initializing the model. This will override both the GPT2 and Anthropic tokenizers.