Skip to content
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

[python] Claude Model Parser #1039

Merged
merged 1 commit into from
Jan 29, 2024
Merged

[python] Claude Model Parser #1039

merged 1 commit into from
Jan 29, 2024

Conversation

Ankush-lastmile
Copy link
Member

@Ankush-lastmile Ankush-lastmile commented Jan 26, 2024

[python] Claude Model Parser

Model Parser for Claude on AWS Bedrock

Claude on Bedrock can be called with the

  • anthropic_bedrock python library
  • AWS' Boto3 Library
  • Chose to use the anthropic_bedrock library because it has good abstractions and type hints

Claude on Bedrock only supports Text Completions, turn style prompts are not supported

  • Added Claude model parser as a core parser.
  • added dependency on anthropic_bedrock

Testplan

using prompt schema from diff ontop

claude.testplan.mov

Stack created with Sapling. Best reviewed with ReviewStack.

@Ankush-lastmile Ankush-lastmile changed the title {wip} Claude Model Parser [python] Claude Model Parser Jan 26, 2024
@@ -53,6 +54,7 @@
DefaultAnyscaleEndpointParser("AnyscaleEndpoint")
)
ModelParserRegistry.register_model_parser(GeminiModelParser("gemini-pro"), ["gemini-pro"])
ModelParserRegistry.register_model_parser(ClaudeBedrockModelParser(), ["Claude"])
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to figure out model vs model provider but for now just registering under "Claude" as well

# See https://docs.anthropic.com/claude/docs/introduction-to-prompt-design#human--assistant-formatting
completion_data[
"prompt"
] = f"{HUMAN_PROMPT} {resolved_prompt}{AI_PROMPT}"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure if its the best idea to manipulate the prompt inside the model parser


completion_data["stream"] = stream

response = self.client.completions.create(**completion_data) # type: ignore (pyright doesn't understand response object)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

API call will throw If aws credentials are not set. I removed mine locally and tested this manually.

Copy link
Contributor

@rossdanlm rossdanlm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, big ups

Comment on lines 222 to 225
# completion parameters to be used for Claude's Text completion api
# prompt handled separately
# streaming handled separately.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Add uri for easy reference

# prompt handled separately
# streaming handled separately.
supported_keys = {
"max_tokens_to_sample",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note w. Ankush offline: required completion param, we should set default so people don't have to (if not defined in config)

"""
return ExecuteResult(
output_type="execute_result",
data=response.completion,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: add link to confirm this is a text-based output only

output_type="execute_result",
data=response.completion,
execution_count=0,
metadata=response.model_dump(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh wow, pydantic nice thanks Anthropic

for iteration in response:
new_text = iteration.completion

# Reduce
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: remove because "reduce" typically has other connotation to it

options (InferenceOptions): The inference options. Used to determine the stream callback.
"""
accumulated_message = ""
output = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this always gets overridden so delete

Comment on lines +29 to +30
# Client will be set in the run method. This is to avoid having to set the api key in the constructor
self.client = None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Create an issue, mention as an issue because this MP is a little weird having 2 potential sources of API KEYS:

# Note: we don't need to explicitly set the key as long as this is set
# as an env var genai.configure() will pick up the env var
# `GOOGLE_API_KEY`, it's just that we prefer not to call
# `get_api_key_from_environment` multiple times if we don't need to
self.api_key = get_api_key_from_environment(
"GOOGLE_API_KEY", required=False
).unwrap()

This will def come up with the SDK rewrites for MPs

Comment on lines +171 to +174
if options is not None and options.stream is not None:
stream = options.stream
elif "stream" in completion_data:
stream = completion_data["stream"]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice this is cleaner than the other implementations we have

else:
output = construct_output(response) # type: ignore

# rewrite or extend list of outputs?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea because this is a chat based one right? Hmmmm, let's do demo you and I today. For now unblocking and we can fix forward based on our desired behaviour

Comment on lines +213 to +214
# Claude outputs should only ever be string
# format so shouldn't get here, but just being safe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I'm talking about

Model Parser for Claude on AWS Bedrock



Claude on Bedrock can be called with the
- `anthropic_bedrock` python library
- AWS' `Boto3` Library
- Chose to use the `anthropic_bedrock` library because it has good abstractions and type hints

Claude on Bedrock only supports Text Completions, turn style [prompts are not supported](https://docs.anthropic.com/claude/reference/claude-on-amazon-bedrock#:~:text=Messages%20in%20Amazon%20Bedrock)

- Added Claude model parser as a core parser.
- added dependency on `anthropic_bedrock`



## Testplan

using prompt schema from diff ontop

https://github.com/lastmile-ai/aiconfig/assets/141073967/29c1faa7-7d13-412f-8606-9ad556eb1c52
@Ankush-lastmile
Copy link
Member Author

  • Removed "Claude" from default model parser registry as a Model Id, just have parser id for now. Will need to revist.
  • addressed comments, most of them were nits.

Ankush-lastmile added a commit that referenced this pull request Jan 29, 2024
[editor] Claude Bedrock Prompt Schema


## Testplan


https://github.com/lastmile-ai/aiconfig/assets/141073967/29c1faa7-7d13-412f-8606-9ad556eb1c52

---
[//]: # (BEGIN SAPLING FOOTER)
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/1050).
* __->__ #1050
* #1039
@Ankush-lastmile Ankush-lastmile merged commit 94bb0d8 into main Jan 29, 2024
2 checks passed
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants